On 2014-02-27 08:05, Ziad Sawalha wrote:
On Feb 26, 2014, at 11:47 AM, Joe Gordon <[email protected]> wrote:
On Wed, Feb 26, 2014 at 9:05 AM, David Ripton <[email protected]>
wrote:
On 02/26/2014 11:40 AM, Joe Gordon wrote:
This is missing the point about manually enforcing style. If you
pass
the 'pep8' job there is no need to change any style.
In a perfect world, yes.
While there are exceptions to this, this just sounds like being extra
nit-picky.
The current reality is that reviewers do vote and comment based on the
rules in
the hacking guide. In terms of style, whether my patch gets approved
or not depends
on who reviews it.
I think that clarifying this in the hacking guide and including it in
our test suites
would remove the personal bias from it. I don’t see folks complaining
that Jenkins is
being nit-picky as a reviewer.
The important aspect here is the mindset, if we don't gate
on style rule x, then we shouldn't waste valuable human review time
and patch revisions on trying to manually enforce it (And yes there
are exceptions to this).
In the real world, there are several things in PEP8 or our project
guidelines that the tools don't enforce perfectly. I think it's fine
for
human reviewers to point such things out. (And then submit a patch
to
hacking to avoid the need to do so in the future.)
To clarify, we don't rely on long term human enforcement for something
that a computer can do.
Precisely. I’m offering to include this as a patch to hacking or even a
separate
test that we can add (without gating initially).
I'm fine with this personally, although it should probably be noted that
we have some PEP 257-type style checks in hacking that get almost
universally ignored because no project has been following those style
guidelines up to this point and it's a significant amount of effort to
fix that. That said, a check for the trailing blank line would be
easier to fix so it might get more traction.
-Ben
_______________________________________________
OpenStack-dev mailing list
[email protected]
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev