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

Doug Hellmann doug at doughellmann.com
Fri Apr 24 14:23:35 UTC 2015

Excerpts from Julien Danjou's message of 2015-04-24 10:14:38 +0200:
> Hi there,
> This is now happening weekly to me now, probably because I write too
> many patches touching almost all OpenStack projects once a cycle, and
> I'm really tired of that behavior, so PLEASE:
>   *Stop sending Code-Review-1 when asking a question in a patch*
> _Sometimes_ there are good reasons to set -1 even when asking a
> question. For example, when the question is a hint sent to the patch
> author so that (s)he improves is commit message, a code comment or a
> piece of code.
> But most of the time, if you ask a question because there's something
> YOU DO NOT KNOW OR UNDERSTAND, do not put a score to a patchset. You
> don't know the answer, so you have absolutely no right to evaluate a
> patchset with -1. Just don't set a score, it's OK, and wait for the
> answer before deciding if the patch is worth [-1..+2].
> Thank you for listening, and happy hacking!

In defense of those of us asking questions, I'll just point out
that as a core reviewer I need to be sure I understand the intent
and wide-ranging ramifications of patches as I review them.  Especially
in the Oslo code, what appears to be a small local change can have
unintended consequences when the library gets out into the applications.

I will often ask questions like, "what is going to happen in X
situation if we change this default" or "how does this change in
behavior affect the case where Y happens, which isn't well tested
in our unit tests." If those details aren't made clear by the commit
message and comments in the code, I consider that a good reason to
include a -1 with a request for the author to provide more detail.
Often these are cases I'm not intimately familiar with, so I ask a
question rather than saying outright that I think something is
broken because I expect to learn from the answer but I still have
doubts that I want to indicate with the -1.

Most of the time the author has thought about the issues and worked
out a reason they are not a problem, but they haven't explained
that anywhere. On the other hand, it is frequently the case that
someone *hasn't* understood why a change might be bad and the
question ends up leading to more research and discussion.


More information about the OpenStack-dev mailing list