pylint:disable deprecated. New oca-disable not working?

[LoisRForgeFlow]

Thanks for the continuous improvement year after year!

One question though. I am updating the ddmrp repository (branch 14.0 - https://github.com/OCA/ddmrp/pull/332) and when running pre-commit I found the following deprecation warning:

ddmrp/ddmrp_purchase_hide_onhand_status/views/purchase_order_view.xml:2 WARNING. DEPRECATED. Use oca-disable instead.

However, as you can see in the PR, doing that replacement does not seem to work as now the xml-view-dangerous-replace-low-priority raises anyway: https://github.com/OCA/ddmrp/actions/runs/6851952066/job/18629369432?pr=332#step:7:101

As you can see in the specific file, there is a reason to disable it: https://github.com/OCA/ddmrp/pull/332/commits/00f030c05e1c2917365a531494b3d69b3a7a515f#diff-b72aa136a90dd553777d52517e3063e85a751b083d38f277441559cec152ed1cR5

Anything else, I should do in order to disable that lint only in this file?

Thanks for your help.

[LoisRForgeFlow]

Hi @sbidoul do you have any quick pointer in this one?

[sbidoul]

Hi Lois, I've not had time to look into this yet. Maybe the docs of https://github.com/OCA/odoo-pre-commit-hooks can help.

[LoisRForgeFlow]

@sbidoul Thanks for the pointer!

I did not realize that there are two hooks doing similar jobs. What I found:

• https://github.com/OCA/odoo-pre-commit-hooks#skip-one-xml-check-for-only-one-file states that the syntax <!-- oca-hooks:disable=xml-check-to-skip --> should disable a check. This is not working as per my testing, so I temporarily disable that via command argument
• pylint_odoo does the same check, and in this case the syntax <!-- pylint:disable=dangerous-view-replace-wo-priority --> still works so I kept it.

I think this solution will allow me to move forward, however there is still the issue oca-hooks:disable syntax.

[sbidoul]

We should not have the same check in both hooks. It looks like our pylint-odoo + odoo-pre-commit-hooks configs need an in-depth review, as I'm afraid we could not keep up with the recent changes. @moylop260 can you help with that?

[antonag32]

Not moylop260, but I can help.

odoo-pre-commit-hooks was created to perform all the non-python checks pylint-odoo was performing (like XML checks). Once odoo-pre-commit-hooks was released, all non-python checks from pylint-odoo were removed.

This means that you are using an old version of pylint-odoo that was released before odoo-pre-commit-hooks (7.0.2). I cloned the ddmrp repository and after updating the hook's version to v9.0.1 there are no more XML checks by Ptython and no more duplicate messages.

As for oca-hooks:disable not working, indeed it seems like that was a bug. Based on my analysis it was fixed with this commit: https://github.com/OCA/odoo-pre-commit-hooks/commit/0314fdf3885a9c0c90ab01f193c1fb8404c50574

So using the latest version should fix it (v0.0.29). The latest version also adds a new check: po-pretty-format, you can disable it using args (as I did) or through an .oca-hooks.cfg on your repository's root.

Overall, the errors you report were fixed with this diff (at least on my end):

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index d72a7bc..d742386 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -52,13 +52,11 @@ repos:
           - --if-source-changed
           - --keep-source-digest
   - repo: https://github.com/OCA/odoo-pre-commit-hooks
-    rev: v0.0.25
+    rev: v0.0.29
     hooks:
       - id: oca-checks-odoo-module
-        # While https://github.com/OCA/oca-addons-repo-template/issues/233
-        # is not fixed.
-        args: ["--disable=xml-view-dangerous-replace-low-priority"]
       - id: oca-checks-po
+        args: ["--disable=po-pretty-format"]
   - repo: https://github.com/myint/autoflake
     rev: v1.4
     hooks:
@@ -145,7 +143,7 @@ repos:
         name: flake8
         additional_dependencies: ["flake8-bugbear==20.1.4"]
   - repo: https://github.com/OCA/pylint-odoo
-    rev: 7.0.2
+    rev: v9.0.1
     hooks:
       - id: pylint_odoo
         name: pylint with optional checks

Try it and let me know how it goes.

[moylop260]

We should not have the same checks in both hooks. Our pylint-odoo + odoo-pre-commit-hooks configs need an in-depth review, as I'm afraid we could not keep up with the recent changes. @moylop260 can you help with that?

Check the following description https://github.com/OCA/pylint-odoo/pull/396

All the pylint-odoo checks not related to python were removed as @antonag32 commented

They were migrated to oca-hooks (e.g. XML, PO...)

It means, that pylint-odoo < 8 has the same checks XML, PO checks than oca-hooks together

That is the reason if you use pylint-odoo < 8 + oca-hooks it will get wrong results because they together are not compatibles

You need to use pylint-odoo >= 8 if you want to use oca-hooks

Notice the following comment related with green results only using the correct versions:

• https://github.com/OCA/account-fiscal-rule/pull/383#discussion_r1397776981

[LoisRForgeFlow]

Hi @antonag32 and @moylop260

Sorry, I for the late reply I typically miss messages when not directly pinged.

I tested your diff and it works, the questions is now: should we update those versions in the template? cc @sbidoul

If you think that is correct, I can do the PR.

Thanks!

[github-actions[bot]]

There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this issue to never become stale, please ask a PSC member to apply the "no stale" label.