Open Stack

> On Apr 24, 2015, at 06:27, Brant Knudson <blk at acm.org> wrote:
> 
> 
> 
>> On Fri, Apr 24, 2015 at 3:14 AM, Julien Danjou <julien at danjou.info> wrote:
>> 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].
> 
> If other developers can't understand your code then the code should be changed to be clearer. There's no reason for OpenStack code to be so complex that a developer can't understand it. Having code that developers don't understand leads to bugs and sometimes security vulnerabilties that affect our users.
> 
> There's also be a corollary to this -- don't +1 (and especially +2) code that you don't understand. In the past I've ignored changes that I wasn't going to understand, typically this has been because it's implementing an external standard that I'm not familiar with and don't want to spend hours reading the spec. These changes tend to sit around unreviewed for a long time, so it would help the submitter to make the code and reasoning clearer. So there's a flip side to this -- if reviewers are going to have trouble understanding the change then expect it to take a long time to merge, if it ever does.
> 

This is really an important reason why -1 with a question cannot be simply "not done". If I don't understand the code, or what will happen in a specific case, a -1 is more useful than a no score. Complex code (or really clever code) is hard to maintain. 

--Morgan


> Maybe it's worth it to have a job that removes -1s after some time, so the reviewer will know to go back and check on it.
> 
> - Brant
> 
> __________________________________________________________________________
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150424/71a436f1/attachment.html>