[OpenStack-Infra] Puppet lint checks for system-config

Yolanda Robla Mota yolanda.robla-mota at hp.com
Thu Aug 13 09:04:32 UTC 2015


I agree with you Jim, but sadly it's a practice in the reality to -1 for
nitpicks or even for questions.
I'd prefer if that nitpicks can be flagged, but as a comment, so they
can be corrected in future iterations if the patch needs more work,
but not a blocking -1.
Also same as -1 for questions, it's really a problem sometimes, because
even if you answer the question, that -1 remains there until the people
who asked reviews again, and prevents changes to land.
I admit I also do that sometimes, because I have the perception that
if i just add a coment and not flag with -1 or +1, i get less attention
from the person being reviewed. We should find ways to solve it.

So i'm glad that this conversation raised, and that this improves us
on review quality and speed.

Best
Yolanda

El 12/08/15 a las 18:00, James E. Blair escribió:
> Paul Belanger <pabelanger at redhat.com> writes:
>
> ...
>> However, recently. I got my hand smacked in 2 different code reviews for arrow
>> alignment issues. Honestly, I wasn't even mad about the -1 for the alignment.
>> However, I'm concerned about the wasted effort the -1 caused me. Basically, I
>> had to wait a few days to get the -1, since it was a human doing the review, not
>> the gate. Additionally, if I was getting a -1 for style checks, why didn't
>> jenkins do it?
> To me, this is the crux of the problem.  It's okay for us to have a
> conversation about whether we want to change the lint rules for
> system-config, but for the moment, we have chosen to have them
> disabled (let's set that conversation aside for now).
>
> Consider that when you leave a -1 on a code review, you are asking a
> contributor to go back and re-do their work.  You are also asking other
> reviewers to re-review a change.  It's a lot of work, and to do it
> frivolously means we are wasting peoples' time.  So, please, when you
> think about whether to leave a -1, don't think "Aha!  I found something
> that could be improved!", instead think "I found something, but is it
> worth redoing already completed work?"
>
> In particular, many of us have a very strong aversion to nitpicking
> about whitespace.  In many cases, we are not in universal agreement on
> what constitutes the most aesthetically pleasing indentation.  In these
> cases, we often loosen the enforcement of our tools so that a wider
> variety of possibilities is accepted.  If we've made such a choice,
> consider embracing the philosophy of IDIC and accepting that there may
> be more than one way to wrap a line.
>
> I have proposed a change to system-config to document what we look for
> in code-review, and what should and should not constitute a -1, so that
> new folks have some background:
>
> https://review.openstack.org/#/c/212073/
>
> -Jim
>
> _______________________________________________
> OpenStack-Infra mailing list
> OpenStack-Infra at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra

-- 
Yolanda Robla Mota
Cloud Automation and Distribution Engineer
+34 605641639
yolanda.robla-mota at hp.com




More information about the OpenStack-Infra mailing list