openerp-community team mailing list archive

[Sandy Carter <sandy.carter@xxxxxxxxxxxxxxxxxxxx>]

Edit: Proposal has been added to the wiki page

https://github.com/OCA/maintainer-quality-tools/wiki

Feedback is appreciated.

Le 2014-11-13 11:42, Sandy Carter a écrit :
> 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
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-community
> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>

Attachment: signature.asc
Description: OpenPGP digital signature