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

[ADD] checkers: category-allowed #459

Merged

Conversation

ThomasBinsfeld
Copy link
Contributor

No description provided.

@moylop260
Copy link
Collaborator

moylop260 commented May 24, 2023

Thank you!

I like this new lint since the default values are empty so it could be enabled explicitly but disable implicitly

So 👍

@luisg123v could you review it, please?

"type": "csv",
"metavar": "<comma separated values>",
"default": DFTL_CATEGORY_ALLOWED,
"help": "List of category allowed in manifest file, separated by a comma.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"help": "List of category allowed in manifest file, separated by a comma.",
"help": "List of allowed categories in manifest file, separated by commas.",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -1007,6 +1019,17 @@ def visit_dict(self, node):
if license_str and license_str not in self.linter.config.license_allowed:
self.add_message("license-allowed", node=manifest_keys_nodes.get("license") or node, args=(license_str,))

# Check category allowed
category_str = manifest_dict.get("category", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Superfluous default value for get

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@moylop260
Copy link
Collaborator

Also, could you add a unittest, please?

@luisg123v
Copy link
Contributor

  • Missing commit description.
  • Where would the default allowed categories be set?

@moylop260
Copy link
Collaborator

@ThomasBinsfeld

Will you able to re-take this work?

@sbidoul
Copy link
Member

sbidoul commented Sep 29, 2023

@ThomasBinsfeld gentle nudge

@ThomasBinsfeld ThomasBinsfeld force-pushed the add_pylint_odoo_checker_category_allowed_tbi branch 2 times, most recently from ce6aa80 to e41a5cf Compare October 27, 2023 08:24
New check allowing to enforce the allowed Odoo modules categories.
A list of coma-separated categories can be set under the category-allowed setting.
@ThomasBinsfeld ThomasBinsfeld force-pushed the add_pylint_odoo_checker_category_allowed_tbi branch from e41a5cf to f8b5c6c Compare October 27, 2023 08:47
@ThomasBinsfeld
Copy link
Contributor Author

Branch rebased and tests added ✔️

@moylop260 moylop260 merged commit 6ab5c34 into OCA:main Nov 7, 2023
15 checks passed
@sbidoul sbidoul deleted the add_pylint_odoo_checker_category_allowed_tbi branch November 8, 2023 09:08
@ThomasBinsfeld
Copy link
Contributor Author

Thanks!

@moylop260
Copy link
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants