Open Stack

There have been many replies on this thread, I'll just reply to this one rather than trying to reply piecemeal.

Doug, there's asking a question because something is unclear (implying that the code is needlessly complex, missing a comment, is unintuitive, ...). I believe that this most definitely warrants a -1 as you describe because it is indicative that the code, once submitted, would be hard for a future reader to follow.

In my mind, the yardstick has been, and continues to be this; would a reasonable person believe that the patch set as presented should be allowed to merge. 

- If I can answer that question unambiguously, and unequivocally with a NO, then I will score the patch with a negative score. 

- If I can answer that question unambiguously, and unequivocally with a YES, then I will score the patch with a positive score. 

- For anything else, I'll use a 0.

If there was a patch to make reviewstats count +0, I'd support that. But I would also like to understand what the differences are between reviewstats and stackalytics. Ideally I would like stackalytics to count 0's as well. If you can take the time to review a patch, you should get credit for it (no matter what tool is used to count). 

I would support changes to both reviewstats and stackalytics to do the following.

1. recognizes and gives credit to '0' comments
2. identifies recheck, reverify and similar directives to the CI system and flag them appropriately, potentially as not being reviews but something else. 

Thanks for reading this far,

-amrith 

| -----Original Message-----
| From: Doug Hellmann [mailto:doug at doughellmann.com]
| Sent: Friday, April 24, 2015 10:24 AM
| To: openstack-dev
| Subject: Re: [openstack-dev] Please stop reviewing code while asking
| questions
| 
| 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.
| 
| Doug
| 
| __________________________________________________________________________
| 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