Hello community, I am writing for two reasons. One is to inform you that there has been a rather drastic change to what Travis tests in code quality. Second is a proposal for a more sensible approach to changes to automatic tests.
==== Firstly, there are new checks done by pylint [1]: (all these can be seen with pylint2 --help-msg <msg-id>[,<msg-id>]) * W0104 pointless-statement: Used when a statement doesn't have (or at least seems to) any effect. * W0105 pointless-string-statement: Used when a string is used as a statement (which of course has no effect). * W0109 duplicate-key: Used when a dictionary expression binds the same key multiple times. * W0403 relative-import: Used when an import relative to the package directory is detected. * W0404 reimported: Used when a module is reimported multiple times. * W0621 redefined-outer-name: Used when a variable's name hide a name defined in the outer scope. * W0622 redefined-builtin: Used when a variable or function override a built-in. This adds on to the current tests: * E0101 return-in-init: Used when the special class method __init__ has an explicit return value. * E1124 redundant-keyword-arg: Used when a function call would result in assigning multiple values to a function parameter, one value from a positional argument and one from a keyword argument. * E1306 too-few-format-args: Used when a format string that uses unnamed conversion specifiers is given too few arguments * I0013 file-ignored: Used to inform that the file will not be checked * W0101 unreachable: Used when there is some code behind a "return" or "raise" statement, which will never be accessed. * W0102 dangerous-default-value: Used when a mutable value as list or dictionary is detected in a default value for an argument. * W1111 assignment-from-none: Used when an assignment is done on a function call but the inferred function returns nothing but None. And Odoo specific tests (called odoo-lint): * EO001 print statement used: Used when there is a call to print The rules for flake8 are: * max-line-length = 79 * ignore unused imports in __init__.py I don't think anyone has gone out and documented this yet, and this is just a quick glance at the confs on my part. These rules should be put somewhere, I suggest the wiki of oca/maintainer-quality-tools. ==== Secondly, the approach to changing these checks needs to be revised. Here is my proposal: We created an organization called oca-travis[2] with this org, pro-active fixes to repos can be done without affecting ongoing projects on oca before merging the new changes to the rules. 1. Before adding new rules, make sure a majority of projects are green (see the controlboard https://github.com/OCA/maintainer-quality-tools/pull/48). If they aren't, an effort should be done to get these tests to work with the current rules. 2. Make sure the community is informed about the new rules. I am talking an email not an obscure PR or Issue that only the most hardcore reviewers will see. 3. Make a PR 4. Accept PR with 3 people, do not merge yet. 5. Give a deadline before the new changes take effect, so that repos can be ready 6. The ones proposing a new rule, _must_ participate in fixing the errors _pro-actively_(ie. code sprint), a script to fork all repos in the org oca-travis and test them using a the MQT from PRs should not be very complicated to write. 7. Close to the deadline, audit the repos on oca-travis and see if the deadline should be extended. 8. Remind the community 9. Merge PR on MQT and pylint fixes from oca-travis 10. Update the wiki to document these new rules. At this point when the community complain, point to the emails and show that procedure was followed. Is this sensible? [1] https://github.com/OCA/maintainer-quality-tools/pull/127 [1] https://github.com/orgs/oca-travis
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Mailing list: https://launchpad.net/~openerp-community Post to : [email protected] Unsubscribe : https://launchpad.net/~openerp-community More help : https://help.launchpad.net/ListHelp
