[openstack-dev] coding standards question
I got a -1 on a review for a standards violation that isn't caught by the automated checks, so I was wondering why the automated check doesn't catch it. The violation was: from X import Y, Z According to the coding standards page on the openstack wiki, the coding standards are PEP8 (they just link to the PEP8 docs): https://wiki.openstack.org/wiki/CodingStandards and PEP8 explicitly says this format is allowed. It was pointed out that there's an additional wiki page I had missed, http://docs.openstack.org/developer/hacking/ which specifies this rule. So now that I see it is a rule, it comes back to my original question, why is it not enforced by the checker? Apparently there's not a flake8 rule for this either https://flake8.readthedocs.org/en/2.0/warnings.html So, two questions: 1. Is this really the rule or just a vaguely worded repeat of the PEP8 rule about import X, Y? 2. If it is the rule, what's involved in getting the pep8 tests to check for it? My own personal frustration aside, this would be helpful for other newcomers I imagine. We have some pretty rigid and extensive coding standards, so its not reasonable to expect new contributors to remember them all. It's also much nicer to have an automated tool tell you you violated some coding standard than to think you were ok and then have your code rejected 2 weeks later because of it. Thanks, Greg P.S. I can fix the wiki to point to the right page after the discussion. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] coding standards question
On 01/07/2014 09:26 AM, Greg Hill wrote: I got a -1 on a review for a standards violation that isn't caught by the automated checks, so I was wondering why the automated check doesn't catch it. The violation was: from X import Y, Z According to the coding standards page on the openstack wiki, the coding standards are PEP8 (they just link to the PEP8 docs): https://wiki.openstack.org/wiki/CodingStandards and PEP8 explicitly says this format is allowed. It was pointed out that there's an additional wiki page I had missed, http://docs.openstack.org/developer/hacking/ which specifies this rule. So now that I see it is a rule, it comes back to my original question, why is it not enforced by the checker? Apparently there's not a flake8 rule for this either https://flake8.readthedocs.org/en/2.0/warnings.html So, two questions: 1. Is this really the rule or just a vaguely worded repeat of the PEP8 rule about import X, Y? 2. If it is the rule, what's involved in getting the pep8 tests to check for it? Writing the hacking test to support it - https://github.com/openstack-dev/hacking The policy leads the automatic enforcement scripts, so there are plenty of rules in the policy that are not in automatic enforcement. Contributions to fill in the gaps are welcomed. My own personal frustration aside, this would be helpful for other newcomers I imagine. We have some pretty rigid and extensive coding standards, so its not reasonable to expect new contributors to remember them all. It's also much nicer to have an automated tool tell you you violated some coding standard than to think you were ok and then have your code rejected 2 weeks later because of it. Thanks, Greg P.S. I can fix the wiki to point to the right page after the discussion. Agreed, it's all about bandwidth. Contributors on hacking to help fill it out are appreciated. Right now it's mostly just Joe with a few others throwing in when they can. -Sean -- Sean Dague Samsung Research America s...@dague.net / sean.da...@samsung.com http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] coding standards question
Thanks Sean. I'll work on adding this one to the hacking repo. That brings up another question, though, what are the implications of suddenly enforcing a rule that wasn't previously enforced? I know there are at least 30 other violations of this rule just within trove, and I imagine larger projects probably have more. I'd hate to be the target of all the ire that sudden rejections of every commit would cause. Do we have a way to make it off by default for some period to let the projects all clean themselves up then turn it on by default after that? Or we could just loosen the coding standards, but that's just crazy talk :D Greg On Jan 7, 2014, at 8:46 AM, Sean Dague s...@dague.net wrote: On 01/07/2014 09:26 AM, Greg Hill wrote: I got a -1 on a review for a standards violation that isn't caught by the automated checks, so I was wondering why the automated check doesn't catch it. The violation was: from X import Y, Z According to the coding standards page on the openstack wiki, the coding standards are PEP8 (they just link to the PEP8 docs): https://wiki.openstack.org/wiki/CodingStandards and PEP8 explicitly says this format is allowed. It was pointed out that there's an additional wiki page I had missed, http://docs.openstack.org/developer/hacking/ which specifies this rule. So now that I see it is a rule, it comes back to my original question, why is it not enforced by the checker? Apparently there's not a flake8 rule for this either https://flake8.readthedocs.org/en/2.0/warnings.html So, two questions: 1. Is this really the rule or just a vaguely worded repeat of the PEP8 rule about import X, Y? 2. If it is the rule, what's involved in getting the pep8 tests to check for it? Writing the hacking test to support it - https://github.com/openstack-dev/hacking The policy leads the automatic enforcement scripts, so there are plenty of rules in the policy that are not in automatic enforcement. Contributions to fill in the gaps are welcomed. My own personal frustration aside, this would be helpful for other newcomers I imagine. We have some pretty rigid and extensive coding standards, so its not reasonable to expect new contributors to remember them all. It's also much nicer to have an automated tool tell you you violated some coding standard than to think you were ok and then have your code rejected 2 weeks later because of it. Thanks, Greg P.S. I can fix the wiki to point to the right page after the discussion. Agreed, it's all about bandwidth. Contributors on hacking to help fill it out are appreciated. Right now it's mostly just Joe with a few others throwing in when they can. -Sean -- Sean Dague Samsung Research America s...@dague.net / sean.da...@samsung.com http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] coding standards question
Yes, you turn it off. Once the change to enforce this rule is merged into hacking, other projects can start refresh their hacking dependency (e.g. upgrading to latest version). The patch to update the dependency has to turn the newly added check off and then consecutive patches can fix all violations in that project and then turn the rule back on. On Tue, Jan 7, 2014 at 11:19 PM, Greg Hill greg.h...@rackspace.com wrote: Thanks Sean. I'll work on adding this one to the hacking repo. That brings up another question, though, what are the implications of suddenly enforcing a rule that wasn't previously enforced? I know there are at least 30 other violations of this rule just within trove, and I imagine larger projects probably have more. I'd hate to be the target of all the ire that sudden rejections of every commit would cause. Do we have a way to make it off by default for some period to let the projects all clean themselves up then turn it on by default after that? Or we could just loosen the coding standards, but that's just crazy talk :D Greg On Jan 7, 2014, at 8:46 AM, Sean Dague s...@dague.net wrote: On 01/07/2014 09:26 AM, Greg Hill wrote: I got a -1 on a review for a standards violation that isn't caught by the automated checks, so I was wondering why the automated check doesn't catch it. The violation was: from X import Y, Z According to the coding standards page on the openstack wiki, the coding standards are PEP8 (they just link to the PEP8 docs): https://wiki.openstack.org/wiki/CodingStandards and PEP8 explicitly says this format is allowed. It was pointed out that there's an additional wiki page I had missed, http://docs.openstack.org/developer/hacking/ which specifies this rule. So now that I see it is a rule, it comes back to my original question, why is it not enforced by the checker? Apparently there's not a flake8 rule for this either https://flake8.readthedocs.org/en/2.0/warnings.html So, two questions: 1. Is this really the rule or just a vaguely worded repeat of the PEP8 rule about import X, Y? 2. If it is the rule, what's involved in getting the pep8 tests to check for it? Writing the hacking test to support it - https://github.com/openstack-dev/hacking The policy leads the automatic enforcement scripts, so there are plenty of rules in the policy that are not in automatic enforcement. Contributions to fill in the gaps are welcomed. My own personal frustration aside, this would be helpful for other newcomers I imagine. We have some pretty rigid and extensive coding standards, so its not reasonable to expect new contributors to remember them all. It's also much nicer to have an automated tool tell you you violated some coding standard than to think you were ok and then have your code rejected 2 weeks later because of it. Thanks, Greg P.S. I can fix the wiki to point to the right page after the discussion. Agreed, it's all about bandwidth. Contributors on hacking to help fill it out are appreciated. Right now it's mostly just Joe with a few others throwing in when they can. -Sean -- Sean Dague Samsung Research America s...@dague.net / sean.da...@samsung.com http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Regards Huang Zhiteng ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] coding standards question
On 01/07/2014 10:19 AM, Greg Hill wrote: Thanks Sean. I'll work on adding this one to the hacking repo. That brings up another question, though, what are the implications of suddenly enforcing a rule that wasn't previously enforced? I know there are at least 30 other violations of this rule just within trove, and I imagine larger projects probably have more. I'd hate to be the target of all the ire that sudden rejections of every commit would cause. Do we have a way to make it off by default for some period to let the projects all clean themselves up then turn it on by default after that? New rules only get released as part of new semver bumps on hacking, and all the projects are pinned on their upper bound on hacking. i.e. hacking=0.8.0,0.9 So new rules would be going into the 0.9.x release stream at this point. Once 0.9.0 is released, we'll up the global requirements. Then projects should update their pins, and either address the issues, or add an ignore for the rules they do not want to enforce (either by policy, or because now is not a good time to fix them). So it is minimally disruptive. -Sean -- Sean Dague http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] coding standards question
So it turns out that trove just has this rule disabled. At least I now know more about how this stuff works, I guess. Sorry for the confusion. Greg On Jan 7, 2014, at 9:54 AM, Sean Dague s...@dague.net wrote: On 01/07/2014 10:19 AM, Greg Hill wrote: Thanks Sean. I'll work on adding this one to the hacking repo. That brings up another question, though, what are the implications of suddenly enforcing a rule that wasn't previously enforced? I know there are at least 30 other violations of this rule just within trove, and I imagine larger projects probably have more. I'd hate to be the target of all the ire that sudden rejections of every commit would cause. Do we have a way to make it off by default for some period to let the projects all clean themselves up then turn it on by default after that? New rules only get released as part of new semver bumps on hacking, and all the projects are pinned on their upper bound on hacking. i.e. hacking=0.8.0,0.9 So new rules would be going into the 0.9.x release stream at this point. Once 0.9.0 is released, we'll up the global requirements. Then projects should update their pins, and either address the issues, or add an ignore for the rules they do not want to enforce (either by policy, or because now is not a good time to fix them). So it is minimally disruptive. -Sean -- Sean Dague http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
