[openstack-dev] Please stop reviewing code while asking questions

John Griffith john.griffith8 at gmail.com
Sat Apr 25 15:25:28 UTC 2015


On Sat, Apr 25, 2015 at 6:28 AM, Jay Pipes <jaypipes at gmail.com> wrote:

> On 04/24/2015 04:33 PM, Ryan Brown wrote:
>
>> On 04/24/2015 03:00 PM, Julien Danjou wrote:
>>
>>> I like that point and I agree with you. The problem, as someone already
>>> stated, is that these people are rarely on IRC and sometimes just never
>>> reply on the review. Right, maybe next time I'll chase them down via
>>> email. Sometimes I wish we were a little more conservative about who
>>> could do code review, but well.
>>>
>>
>> I'm pretty heavily against limiting who can code review. There are some
>> less-than-helpful reviewers about, but putting up barriers is the wrong
>> way to go about fixing it.
>>
>> Education is the way to go, and it's ok if there's some nominal level of
>> somewhat unhelpful reviews so long as, when possible, we try to teach
>> those reviewers how they can be more helpful.
>>
>
> +1
>
> -jay
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>

​Just my 2 cents, I sometimes give a "0" score when asking questions,
sometimes a -1.  It's subjective based on whether *I* believe it may or may
not have an impact on the code and whether the patch works.  Could be an
oversight by the committer etc.

That being said, people do LOVE hitting that -1 button it seems, which is
annoying at times.

As others have said, the last thing I want to do is discourage reviews of
any sort from anybody.  Personally I want people to ask questions, and
frankly there's three or four people on this list saying how terrible -1
with questions is, but I seem to recall each of them at least once
providing me with a prompt -1 with questions in patches I've submitted
(just saying).  Sometimes it's tedious, sometimes it's down right
infuriating... but there are also times where it makes me think about what
I'm doing rather than just *hacking* out some short term fix on something,
or gives me an idea to make a patch better.

Some may be warranted, some may not... I'd rather error on the side of "it
matters" for the most part.  No offense to anybody but if you have this
happen to you multiple times in a week, over and over, maybe there is
something about your code, commit-messages or comments in your code that
just isn't doing it for people?  Or maybe it's crap... I don't know, but
again might be good to keep an open mind about it.

Also, personally this sort of thing is why I avoid the -1 filters when
looking at code reviews.  Now if I open the review that has a -1 and see
it's from somebody I have a good deal of faith in, I *might* ignore it, but
even then I want to see what they "found", and maybe improve my own
reviewing/commit habits.  Kinda dangerous IMO that we're sort of developing
a clique, or belief that anybody is infallible, and that we shouldn't have
to communicate or explain our work to anybody.

On that note, maybe folks on this list should go look at their review inbox
and make sure they're not guilty here, and let the change begin with
themselves.

Thanks,
John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150425/8afe6184/attachment.html>


More information about the OpenStack-dev mailing list