Hi Sandy, On 11/13/2014 06:13 PM, Sandy Carter wrote: > 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. do you forbid pdb.set_trace() or the import of pdb and friends as a whole ? If that's the latter, I'd be firmly against it, since it also forbids very useful stuff, such as insertion of (optional, of course) pdb post_mortem calls. Of course if there's a way to bypass it with a # noqa comment or equivalent, this objection does not stand.
> > 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@lists.launchpad.net >> Unsubscribe : https://launchpad.net/~openerp-community >> More help : https://help.launchpad.net/ListHelp >> > > > _______________________________________________ > Mailing list: https://launchpad.net/~openerp-community > Post to : openerp-community@lists.launchpad.net > Unsubscribe : https://launchpad.net/~openerp-community > More help : https://help.launchpad.net/ListHelp
_______________________________________________ Mailing list: https://launchpad.net/~openerp-community Post to : openerp-community@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community More help : https://help.launchpad.net/ListHelp