[Openerp-community] Updates to Travis and maintainer-quality-tools

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[Openerp-community] Updates to Travis and maintainer-quality-tools

Sandy Carter (http://www.savoirfairelinux.com)
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     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Updates to Travis and maintainer-quality-tools

Sandy Carter (http://www.savoirfairelinux.com)
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     : [hidden email]
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[Openerp-community] Proposal to add W0402 to Travis Style checking

Sandy Carter (http://www.savoirfairelinux.com)
In reply to this post by Sandy Carter (http://www.savoirfairelinux.com)
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     : [hidden email]
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Proposal to add W0402 to Travis Style checking

Georges Racinet
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     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp


      

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Updates to Travis and maintainer-quality-tools

Daniel Reis (SECURITAS SA)-3
In reply to this post by Sandy Carter (http://www.savoirfairelinux.com)
Thank you for bringing up this discussion Sandy.

I believe this should be put in perspective with previous decisions on
the same topic.
For reference, these are the relevant discussions:

*  Releases and announcements #84:
https://github.com/OCA/maintainer-quality-tools/issues/84
*  Quality Tools Releases 1 and 2 Announcement #86:
https://github.com/OCA/maintainer-quality-tools/issues/86

Regards
Daniel

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Proposal to add W0402 to Travis Style checking

Georges Racinet
In reply to this post by Georges Racinet
On 11/13/2014 06:21 PM, Georges Racinet wrote:
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.

Forgot to conclude with what is obviously needed :
thank you and other people working on improving the quality ! This is incredibly positive.

Regards,



_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Proposal to add W0402 to Travis Style checking

Moises Lopez
In reply to this post by Georges Racinet
Hello Georges,

Sentence import pdb, import pudb, import ipdb (and maybe print) is usefully to debug local.
If you need use this sentence in stable branch you will can add a comment of this type:

import pdb # pylint: disable=W0402


2014-11-13 11:21 GMT-06:00 Georges Racinet <[hidden email]>:
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     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp


      

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp




--
Moisés López Calderón
Vauxoo - OpenERP's Gold Partner
Mobile: (+521) 477-752-22-30
Office: (+52) 477-773-33-46
web: http://www.vauxoo.com
twitter: @vauxoo
           @moylop260           


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Proposal to add W0402 to Travis Style checking

Sandy Carter (http://www.savoirfairelinux.com)
In reply to this post by Georges Racinet
I think the PR could be expanded to add/remove deprecated modules from
the .travis file.

The reason for this is that for certain branches openerp.osv.osv is
acceptable and on the newer ones, they have become deprecated.

There may be truth that the call to set_trace should be the one blocked.
Please raise your objection on the merge proposal.

Le 2014-11-13 12:24, Georges Racinet a écrit :

> On 11/13/2014 06:21 PM, Georges Racinet wrote:
>> 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.
>
> Forgot to conclude with what is obviously needed :
> thank you and other people working on improving the quality ! This is
> incredibly positive.
>
> Regards,
>
>
>
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-community
> Post to     : [hidden email]
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Updates to Travis and maintainer-quality-tools

Moises Lopez
In reply to this post by Daniel Reis (SECURITAS SA)-3
Hello Sandy,
+1
IMHO I like more use a .md file into same project. (not wiki)
You can add a new feature in PR and add new doc in same PR.

md file is more portable.
*For your fork
*For you use in other git tool. Example: bitbucket, gitlab
(In same project)

What do you think?

md is same visual that a wiki.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Updates to Travis and maintainer-quality-tools

Sandy Carter (http://www.savoirfairelinux.com)
> IMHO I like more use a .md file into same project. (not wiki)

Wiki is in .md file format.
It is better for this highlevel explanation to be in the wiki, IMHO

> You can add a new feature in PR and add new doc in same PR.
>
> md file is more portable.
> *For your fork
> *For you use in other git tool. Example: bitbucket, gitlab
> (In same project)

The wiki is a github repo that you can clone with
https://github.com/OCA/maintainer-quality-tools.wiki.git

Contents are:

Home.md


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Updates to Travis and maintainer-quality-tools

Moises Lopez
I don't talk about of just a .md file
I'm talking about of a .md file into same project.
If you want add a change to wiki you will need wait that you pr of IMP was merged to start to change wiki.
If you want change wiki doc you will need use other git project:
[hidden email]:OCA/maintainer-quality-tools.wiki.git

If you want review doc before of push (in a PR maybe) you view this one after of changed.

IMHO a change of documentation should be into oca changes flow.
 - Make PR
 - Feedback
 - +1*3
 - Merged

This is posible using the same original project.

Plus: If tomorrow changes to other git tool you have this documentation without additional script.


2014-11-13 11:53 GMT-06:00 Sandy Carter <[hidden email]>:
> IMHO I like more use a .md file into same project. (not wiki)

Wiki is in .md file format.
It is better for this highlevel explanation to be in the wiki, IMHO

> You can add a new feature in PR and add new doc in same PR.
>
> md file is more portable.
> *For your fork
> *For you use in other git tool. Example: bitbucket, gitlab
> (In same project)

The wiki is a github repo that you can clone with
https://github.com/OCA/maintainer-quality-tools.wiki.git

Contents are:

Home.md




--
Moisés López Calderón
Vauxoo - OpenERP's Gold Partner
Mobile: (+521) 477-752-22-30
Office: (+52) 477-773-33-46
web: http://www.vauxoo.com
twitter: @vauxoo
           @moylop260           


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Updates to Travis and maintainer-quality-tools

Moises Lopez
In reply to this post by Sandy Carter (http://www.savoirfairelinux.com)
You can check wiki pages of trending project from github:
And you can check wiki pages is empty.
All documentation is into same project.

Same case for this big organization:


2014-11-13 11:53 GMT-06:00 Sandy Carter <[hidden email]>:
> IMHO I like more use a .md file into same project. (not wiki)

Wiki is in .md file format.
It is better for this highlevel explanation to be in the wiki, IMHO

> You can add a new feature in PR and add new doc in same PR.
>
> md file is more portable.
> *For your fork
> *For you use in other git tool. Example: bitbucket, gitlab
> (In same project)

The wiki is a github repo that you can clone with
https://github.com/OCA/maintainer-quality-tools.wiki.git

Contents are:

Home.md




--
Moisés López Calderón
Vauxoo - OpenERP's Gold Partner
Mobile: (+521) 477-752-22-30
Office: (+52) 477-773-33-46
web: http://www.vauxoo.com
twitter: @vauxoo
           @moylop260           


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Updates to Travis and maintainer-quality-tools

Marc Cassuto (SFL)
+1 Sandy

We are working with semi-automated Q&A process; it is is important to know what do change in the travis rules before we ask for a PR.
I have to say it is particularly frustrating to have PR rejected because the rules changes behind the scene.

IMHO, it raises also the question of the conformity to these rules of all the previously merged code...

My 2 cents,
Marc

==
Marc Cassuto, Ing. MBA Itil

SAVOIR-FAIRE LINUX
Consultant solutions ERP
T: 514-276-5468 #180


From: "Moises Lopez" <[hidden email]>
To: "Sandy Carter" <[hidden email]>
Cc: "openerp-community" <[hidden email]>
Sent: Thursday, November 13, 2014 1:07:58 PM
Subject: Re: [Openerp-community] Updates to Travis and        maintainer-quality-tools

You can check wiki pages of trending project from github:
And you can check wiki pages is empty.
All documentation is into same project.

Same case for this big organization:


2014-11-13 11:53 GMT-06:00 Sandy Carter <[hidden email]>:
> IMHO I like more use a .md file into same project. (not wiki)

Wiki is in .md file format.
It is better for this highlevel explanation to be in the wiki, IMHO

> You can add a new feature in PR and add new doc in same PR.
>
> md file is more portable.
> *For your fork
> *For you use in other git tool. Example: bitbucket, gitlab
> (In same project)

The wiki is a github repo that you can clone with
https://github.com/OCA/maintainer-quality-tools.wiki.git

Contents are:

Home.md




--
Moisés López Calderón
Vauxoo - OpenERP's Gold Partner
Mobile: (+521) 477-752-22-30
Office: (+52) 477-773-33-46
web: http://www.vauxoo.com
twitter: @vauxoo
           @moylop260           


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Proposal to add W0402 to Travis Style checking

Georges Racinet
In reply to this post by Moises Lopez
On 11/13/2014 06:32 PM, Moises Lopez wrote:
Hello Georges,

Sentence import pdb, import pudb, import ipdb (and maybe print) is usefully to debug local.
If you need use this sentence in stable branch you will can add a comment of this type:

import pdb # pylint: disable=W0402

and that is enough for me to lift the objection !
For the record, I've been using pre-commit hooks for years to avoid accidental commits of  pdb.set_trace(), but that wouldn't do for a project wide enforcement such as the one being set in place.

Keep up the great job, folks !




_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] Proposal to add W0402 to Travis Style checking

Leonardo Pistone - camptocamp
Thanks Sandy for the initiative.

I agree we should find a way to be a bit more careful changing the
linting rules, and your proposal looks good to me in general.

A remark on a detail: do we need the extra organization? We could work
the way we do for day-to-day pull requests: a simple PR from a
personal account. If multiple contributors are needed, of course
merges and forks from one another as usual. Or I missed something?

On the other hand, in the future we could maybe handle this change
control of linting differently: make the maintainer-quality-tools a
python package with releases, with releases fixed in per-project
requirements.txt. I see those advantages:

- orthogonal to travis, so I can consistently run the same lint check
locally that is run on travis
- version can be easily fixed, and the PR that greenifies a branch
with a new MQT release, updates also the MQT version used for that
branch in requirements.txt
- as a consequence, we do classic releases (with a changelog and all)
of MQT, and don't need the process you suggest (but I agree the rules
would be, proponents need to check that projects follow the new
release and proactively make PRs to update them to the new MQT)

...but all that is words on my part, it would need some work to make
the python package. So, for the time being, :+1: for your proposal!

On Thu, Nov 13, 2014 at 7:48 PM, Georges Racinet <[hidden email]> wrote:

> On 11/13/2014 06:32 PM, Moises Lopez wrote:
>
> Hello Georges,
>
> Sentence import pdb, import pudb, import ipdb (and maybe print) is usefully
> to debug local.
> If you need use this sentence in stable branch you will can add a comment of
> this type:
>
> import pdb # pylint: disable=W0402
>
> and that is enough for me to lift the objection !
> For the record, I've been using pre-commit hooks for years to avoid
> accidental commits of  pdb.set_trace(), but that wouldn't do for a project
> wide enforcement such as the one being set in place.
>
> Keep up the great job, folks !
>
>
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-community
> Post to     : [hidden email]
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp