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). 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 [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
