openerp-community team mailing list archive
Mailing list archive
Updates to Travis and maintainer-quality-tools
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 :
(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
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
Secondly, the approach to changing these checks needs to be revised.
Here is my proposal:
We created an organization called oca-travis 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
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
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?
Description: OpenPGP digital signature