Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: pylint-odoo v8 configuration options ignored #204

Merged
merged 1 commit into from
Aug 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/.pylintrc-mandatory.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,21 @@ load-plugins=pylint_odoo
score=n

[ODOOLINT]
{%- if odoo_version < 16 %}
readme_template_url="https://github.com/OCA/maintainer-tools/blob/master/template/module/README.rst"
manifest_required_authors={{ org_name }}
manifest_required_keys=license
manifest_deprecated_keys=description,active
license_allowed=AGPL-3,GPL-2,GPL-2 or any later version,GPL-3,GPL-3 or any later version,LGPL-3
valid_odoo_versions={{ odoo_version }}
{%- else %}
readme-template-url="https://github.com/OCA/maintainer-tools/blob/master/template/module/README.rst"
manifest-required-authors={{ org_name }}
manifest-required-keys=license
manifest-deprecated-keys=description,active
license-allowed=AGPL-3,GPL-2,GPL-2 or any later version,GPL-3,GPL-3 or any later version,LGPL-3
valid-odoo-versions={{ odoo_version }}
{%- endif %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution sounds a bit awkward to me. Summoning @moylop260 here: is this the right way to do this? Did pylint-odoo 8 actually change the option names?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on OCA/pylint-odoo#465 thats what I have understood from moylop260 and observed :) If I've made a huge mistake apologies :)

As for the fix I copied the version checking pattern that was being used elsewhere. For changing the tests I explicitly checked the pylint-odoo version. I did toy with doing that, but elected to try and follow the same pattern. I don't mind revisiting this if there's a preferred change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theangryangel thanks for the explanation (which I should have found myself, had I read your OP carefully :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're all busy people :) It's easily done, anything I can do to try and make life easier tho :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for take care about here

BTW I forgot to update this template after that release


[MESSAGES CONTROL]
disable=all
Expand Down
23 changes: 21 additions & 2 deletions tests/test_copy.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from pathlib import Path

import packaging.version
import yaml
from copier.main import run_auto

Expand All @@ -12,6 +13,16 @@
REPO_ID = 149


def read_and_parse_precommit_rev(tmp_path: Path, repo: str):
# Attempt to find the rev of a pre-commit repo.
# If it found, return the actual version, if it's not return 0.0.0.
config = yaml.safe_load((tmp_path / ".pre-commit-config.yaml").read_text())
entry = next(
filter(lambda r: r.get("repo", "") == repo, config.get("repos", [])), {}
)
return packaging.version.parse(entry.get("rev", "0.0.0"))


def test_bootstrap(tmp_path: Path, odoo_version: float, cloned_template: Path):
"""Test that a project is properly bootstrapped."""
data = {
Expand All @@ -31,12 +42,20 @@ def test_bootstrap(tmp_path: Path, odoo_version: float, cloned_template: Path):
pylintrc_mandatory = (tmp_path / ".pylintrc-mandatory").read_text()
assert "disable=all\n" in pylintrc_mandatory
assert "enable=" in pylintrc_mandatory
assert f"valid_odoo_versions={odoo_version}" in pylintrc_mandatory
pylint_odoo_version = read_and_parse_precommit_rev(
tmp_path, "https://github.com/OCA/pylint-odoo"
)
valid_odoo_versions = "valid_odoo_versions"
if pylint_odoo_version >= packaging.version.parse("v8.0.5"):
# versions of pylint-odoo >= 8.0.5 use dashes for keys, rather than
# underscores
valid_odoo_versions = "valid-odoo-versions"
assert f"{valid_odoo_versions}={odoo_version}" in pylintrc_mandatory
assert SOME_PYLINT_OPTIONAL_CHECK not in pylintrc_mandatory
pylintrc_optional = (tmp_path / ".pylintrc").read_text()
assert "disable=all\n" in pylintrc_optional
assert "# This .pylintrc contains" in pylintrc_optional
assert f"valid_odoo_versions={odoo_version}" in pylintrc_optional
assert f"{valid_odoo_versions}={odoo_version}" in pylintrc_optional
assert SOME_PYLINT_OPTIONAL_CHECK in pylintrc_optional
flake8 = (tmp_path / ".flake8").read_text()
assert "[flake8]" in flake8
Expand Down
Loading