search

OCA / odoo-pre-commit-hooks

Linters of Odoo addons that complement pylint-odoo
GNU Affero General Public License v3.0
10 stars 12 forks source link

[FIX] oca_pre_commit_hooks: Do not be so strict with po file #90

Closed etobella closed 1 year ago

etobella commented 1 year ago

This could be used in order to fix this:

https://github.com/OCA/connector/pull/471#issuecomment-1787342916

etobella commented 1 year ago

Finally, it is :green_apple: @moylop260 May I ask for your review?

antonag32 commented 1 year ago

If it is just about excluding files, I think that should be configured on the hook configuration itself, it is a task already performed by pre-commit, and it saves some complexity on hooks themselves.

If we want to fix this upstream (on this repo), we could just change .pre-commit-hooks.yaml.

antonag32 commented 1 year ago

Something like this (untested but you get the idea):

diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml
index bb77ed2..7501937 100644
--- a/.pre-commit-hooks.yaml
+++ b/.pre-commit-hooks.yaml
@@ -21,5 +21,5 @@
   # The command "identify-cli file.pot"
   # returns "text"
   types_or: ["text"]
-  files: \.(po|pot)$
+  files: i18n/.*\.(po|pot)$
   require_serial: false

Also a nitpick of mine but if we are going to update the config file we might as well check if types_or serves any purpose (seems redundant if we are using files)

antonag32 commented 1 year ago

This is the file I am taking about: https://github.com/OCA/odoo-pre-commit-hooks/blob/5e97e13778f2cf7440bc711171f9e4db69a0f6a7/.pre-commit-hooks.yaml#L14-L25

And relevant documentation regarding the files option: https://pre-commit.com/#hooks-files