Open Stack

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