openerp-community team mailing list archive

[Sandy Carter <sandy.carter@xxxxxxxxxxxxxxxxxxxx>]

Following my proposed procedure, I am informing the community that we
intend to add a new style restriction [1].


W0402 deprecated-module:Used a module marked as deprecated is imported.

Modules marked as deprecated: pdb,pudb,ipdb

The reason behind this is that these are developement modules used to
pause code in the middle of execution in order to debug. They are not
intended for production but can be forgotten in the code and commited.
To catch these is necessary since it halt code execution and this is not
desired in production.

The implication of this is that any code with any of these modules will
fail on travis and be rejected by the reviewer.

We await your feedback. If there are no objections, this new rule will
be added to the entirety of OCA repos and any modules using them will
fail the project as a whole and as a result, potentially block all pull
requests until these modules are fixed.

[1] https://github.com/OCA/maintainer-quality-tools/pull/113

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