On Wed, Aug 12, 2015 at 09:00:10AM -0700, James E. Blair wrote: > Paul Belanger <[email protected]> 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). > Ya, this was the initial reason for my post, to revisit the discussion.
> 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/ > Thanks for doing this. > -Jim _______________________________________________ OpenStack-Infra mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
