Re: [openstack-dev] Please stop reviewing code while asking questions
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-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Sat, Apr 25, 2015 at 6:28 AM, Jay Pipes jaypi...@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-requ...@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 __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
To follow up on Robert's post, Gerrit's query command [1] supports the retrieval of comments for each patchset so maybe we just need to add the '--comments' option when requesting changes [2] ? Since I've posted, I'll just provide my own opinion on this topic. I have no problems with a -1 ask a question vote for any reason but I do think that it's irresponsible to _not_ follow up. So it's the follow up that I would like to see fixed not the restriction on reviews because I feel that it would discourage reviews. [1] https://review.openstack.org/Documentation/cmd-query.html [2] https://git.openstack.org/cgit/openstack-infra/reviewstats/tree/reviewstats/utils.py#n170 On Fri, Apr 24, 2015 at 9:04 PM, Robert Collins robe...@robertcollins.net wrote: I believe we'll need to switch to the REST JSON API, which gertty uses and gets comments - some nontrivial refactoring implied, and I shudder to think about the performance impact. I've wanted to rewrite reviewstats for a while... this might be the trigger... -Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] Please stop reviewing code while asking questions
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! -- Julien Danjou ;; Free Software hacker ;; http://julien.danjou.info signature.asc Description: PGP signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
Julien,
We had a similar discussion within Trove several months ago and agreed to a
convention that if you have a question, that should not warrant a -1 unless, as
you indicate there's a strong reason to believe that the code is wrong and the
question is leading.
We discussed this at a mid-cycle and agreed to put our conventions in
CONTRIBUTING.rst[1].
We had a hypothesis about why +0 was rarely used (never conclusively proved).
Our hypothesis was that since Stackalytics didn't count +0's it led to an
increased propensity to -1 something. It would be wonderful if we could try the
experiment of giving credit for 0's and seeing if it changes behavior.
It may be relevant to note that one of the recent candidates for TC also cited
the possibility that a change in stackalytics was a causal factor in the change
in behavior re: commits and reviews[2].
Maybe this is something that the new TC candidates can opine on; are these
kinds of metrics driving bad behavior and if so what, if anything, can the TC
do about it?
-amrith
[1] https://github.com/openstack/trove/blob/master/CONTRIBUTING.rst
{2] http://openstack.markmail.org/thread/2xfapsmyy5i44adj
| -Original Message-
| From: Gorka Eguileor [mailto:gegui...@redhat.com]
| Sent: Friday, April 24, 2015 4:29 AM
| To: openstack-dev@lists.openstack.org
| Subject: Re: [openstack-dev] Please stop reviewing code while asking
| questions
|
| On Fri, Apr 24, 2015 at 10:14:38AM +0200, Julien Danjou 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].
|
| Thank you for listening, and happy hacking!
|
| --
| Julien Danjou
| ;; Free Software hacker
| ;; http://julien.danjou.info
|
| +1
|
| It does bother me too, especially when you answer the question and you
| never hear back from them and the -1 stays there... XD
|
|
| Gorka
|
| __
| OpenStack Development Mailing List (not for usage questions)
| Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
| http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24, 2015 at 10:14:38AM +0200, Julien Danjou 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]. Thank you for listening, and happy hacking! -- Julien Danjou ;; Free Software hacker ;; http://julien.danjou.info +1 It does bother me too, especially when you answer the question and you never hear back from them and the -1 stays there... XD Gorka __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 04/24/2015 10:14 AM, Julien Danjou 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]. Thank you for listening, and happy hacking! Generally +1. Neither reviewers should put -1 due to nitty nits for typos in comments and especially in commit messages. There is a huge price of it (we often note cpu cycles in this regard, but the actual price is frustration and excessive attention to irrelevant issues from fellow contributors). [Sometimes typos in comments and commit messages really deserve -1, f.e. when they are not intelligible due to those typos, but it's one in a million case.] - From the other side, the problem with no score comments is that some contributors assume those comments are not worth attention, since they presumably don't influence the merge process. And (almost) all comments are worth being answered. If not for the particular patchset, if not for the author of the patch, then at least for the reviewer. Ihar -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQEcBAEBCAAGBQJVOhZ9AAoJEC5aWaUY1u57FaoIAMg1cY7Rkif7SRBJdokBQ9Ky R8tSkOPGripNJGxBc7NCfZOTeGFxU6cNI2pSpcEyZ3Gt256al87bWasbu8Drak2k UEU0KsolOwjEa6z9uZ29Q0b3I2bGAIKk17otgsHC4eZ3jjdghO4IFgj1bVXR4kV1 dFX0vJtfaSGF3WEol2B1ZOtyi45fshM2inT/tr0S4qHxuejEvL+QN4IOhRmRcf6Z w2BwtS2khIwj8HOHtsbd9KvQ5UqM8Vv4GQ5+Jz+864TRjZKiFWZFqYt47xyh9/Si ZDXgZID0AdHv3mKtYOxpxhG5WtMUld4C7Ljm9sGKMgdCfLkevVyb3MxzoiEESJ0= =/YE0 -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 04/24/2015 08:30 AM, Sylvain Bauza wrote: Le 24/04/2015 14:11, Russell Bryant a écrit : On 04/24/2015 07:21 AM, Amrith Kumar wrote: We had a hypothesis about why +0 was rarely used (never conclusively proved). Our hypothesis was that since Stackalytics didn't count +0's it led to an increased propensity to -1 something. It would be wonderful if we could try the experiment of giving credit for 0's and seeing if it changes behavior. I think this makes a lot of sense. These stats really do drive behavior. I'd certainly be open to a patch to reviewstats [1] to count +0 comments and I think it would be good for stackalytics to consider the same. [1] http://git.openstack.org/cgit/openstack-infra/reviewstats Just a question I have since a while... I know that Stackalytics and reviewstats are not counting the same things for reviews (reviewstats is definitely better because it counts all the comments, and not just if I'm happy or not with a specific patchset). Could Stackalytics modify its behaviour to mimic reviewstats ? Ideally, Stackalytics should call the reviewstats API and reviewstats should be hosted by infra IMHO (its already in the -infra namespace...) so we could prevent duplicates. There has been talk of moving stackalytics into infra somewhere. They're implemented *very* differently, so it's not really practical for stackalytics to use reviewstats code. Maybe it's good to have 2 anyway, as it helps point out subtle differences that implementations can make, like you've pointed out here. -- Russell Bryant __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
Ive seen this a lot too. It is frustrating. Kevin From: Julien Danjou Sent: Friday, April 24, 2015 1:14:38 AM To: openstack-dev@lists.openstack.org Subject: [openstack-dev] Please stop reviewing code while asking questions 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! -- Julien Danjou ;; Free Software hacker ;; http://julien.danjou.info __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
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-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 24 April 2015 at 14:11, Russell Bryant rbry...@redhat.com wrote: On 04/24/2015 07:21 AM, Amrith Kumar wrote: We had a hypothesis about why +0 was rarely used (never conclusively proved). Our hypothesis was that since Stackalytics didn't count +0's it led to an increased propensity to -1 something. It would be wonderful if we could try the experiment of giving credit for 0's and seeing if it changes behavior. I think this makes a lot of sense. These stats really do drive behavior. I'd certainly be open to a patch to reviewstats [1] to count +0 comments and I think it would be good for stackalytics to consider the same. [1] http://git.openstack.org/cgit/openstack-infra/reviewstats Frankly I think that it is an annoying behaviour to set a score so that your act of asking a question or nit-picking a patch gets counted. Even if internally in project teams we do count these stats, rest assured that we also verify the quality that lies between those numbers. A contributor who does proof-reading of 600 commit messages a month surely won't be promoted to any core team. If you think it might be beneficial to adjust tooling to that these contributions get counted this is fine by me. I just wanted to point out that I do not consider those contributions at all (and btw it would be at least more polite to put a +1 rather than a -1). It is my opinion that the kind of negative scores pointed out by Ihar and Julien should just be ignored. As a core reviewer for Openstack/Neutron I've been actually doing so for a while - I hope now I won't be accused of being community un-friendly ;) Salvatore -- Russell Bryant __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 24/04/15 07:21, Amrith Kumar wrote:
Julien,
We had a similar discussion within Trove several months ago and agreed to a
convention that if you have a question, that should not warrant a -1 unless, as
you indicate there's a strong reason to believe that the code is wrong and the
question is leading.
+1. I'm kind of shocked that this even needed to be discussed, but well
done :)
We discussed this at a mid-cycle and agreed to put our conventions in
CONTRIBUTING.rst[1].
We had a hypothesis about why +0 was rarely used (never conclusively proved).
Our hypothesis was that since Stackalytics didn't count +0's it led to an
increased propensity to -1 something. It would be wonderful if we could try the
experiment of giving credit for 0's and seeing if it changes behavior.
IIRC the problem here is the Gerrit API - it doesn't count +0 as a
'review', so they just don't show up in any automated tools. (This isn't
easily solved either, even assuming you're willing to modify Gerrit.)
There's nothing quite so antisocial as obstructing someone else's work
to juice your own stats, and it's good to (gently) remind everyone of
that occasionally.
It may be relevant to note that one of the recent candidates for TC also cited
the possibility that a change in stackalytics was a causal factor in the change
in behavior re: commits and reviews[2].
Maybe this is something that the new TC candidates can opine on; are these
kinds of metrics driving bad behavior and if so what, if anything, can the TC
do about it?
Individualised closed-loop metrics *always* drive bad behaviour, because
they're necessarily only a sample of the behaviour we care about and to
the extent that sample is representative of the whole, it can only
remain so in the open-loop case. So we can, and should, tweak metrics to
reduce bad behaviour and encourage good behaviour, but we shouldn't kid
ourselves that we can eliminate unintended consequences - we can only
exchange them for _different_ unintended consequences.
This is an open community, so we can't (and shouldn't want to) prevent
people from publishing stats. The best case is that we use them only to
inform us how we're doing in the aggregate, and discourage companies in
particular from attaching individual incentives to game the metrics.
Members of the TC, at least, (I don't know that there was ever an
official edict or anything) have expressed this in the past, and I think
it's one of those things that requires vigilance and periodic reminders.
cheers,
Zane.
-amrith
[1] https://github.com/openstack/trove/blob/master/CONTRIBUTING.rst
{2] http://openstack.markmail.org/thread/2xfapsmyy5i44adj
| -Original Message-
| From: Gorka Eguileor [mailto:gegui...@redhat.com]
| Sent: Friday, April 24, 2015 4:29 AM
| To: openstack-dev@lists.openstack.org
| Subject: Re: [openstack-dev] Please stop reviewing code while asking
| questions
|
| On Fri, Apr 24, 2015 at 10:14:38AM +0200, Julien Danjou 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].
|
| Thank you for listening, and happy hacking!
|
| --
| Julien Danjou
| ;; Free Software hacker
| ;; http://julien.danjou.info
|
| +1
|
| It does bother me too, especially when you answer the question and you
| never hear back from them and the -1 stays there... XD
|
|
| Gorka
|
| __
| OpenStack Development Mailing List (not for usage questions)
| Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
| http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
Le 24/04/2015 14:11, Russell Bryant a écrit : On 04/24/2015 07:21 AM, Amrith Kumar wrote: We had a hypothesis about why +0 was rarely used (never conclusively proved). Our hypothesis was that since Stackalytics didn't count +0's it led to an increased propensity to -1 something. It would be wonderful if we could try the experiment of giving credit for 0's and seeing if it changes behavior. I think this makes a lot of sense. These stats really do drive behavior. I'd certainly be open to a patch to reviewstats [1] to count +0 comments and I think it would be good for stackalytics to consider the same. [1] http://git.openstack.org/cgit/openstack-infra/reviewstats Just a question I have since a while... I know that Stackalytics and reviewstats are not counting the same things for reviews (reviewstats is definitely better because it counts all the comments, and not just if I'm happy or not with a specific patchset). Could Stackalytics modify its behaviour to mimic reviewstats ? Ideally, Stackalytics should call the reviewstats API and reviewstats should be hosted by infra IMHO (its already in the -infra namespace...) so we could prevent duplicates. 2cts over that, -Sylvain __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24, 2015 at 3:14 AM, Julien Danjou jul...@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. 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-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 04/24/2015 07:21 AM, Amrith Kumar wrote: We had a hypothesis about why +0 was rarely used (never conclusively proved). Our hypothesis was that since Stackalytics didn't count +0's it led to an increased propensity to -1 something. It would be wonderful if we could try the experiment of giving credit for 0's and seeing if it changes behavior. I think this makes a lot of sense. These stats really do drive behavior. I'd certainly be open to a patch to reviewstats [1] to count +0 comments and I think it would be good for stackalytics to consider the same. [1] http://git.openstack.org/cgit/openstack-infra/reviewstats -- Russell Bryant __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24, 2015 at 11:16 AM, Julien Danjou jul...@danjou.info wrote: On Fri, Apr 24 2015, Joe Gordon wrote: When I get a -1 on one of my patches with a question, I personally treat it as a short coming of the commit message. To often in the past I have looked at a file, and in trying to figure out why that line is there I do a git blame only to see a useless commit message with me as the author. That's a thing that I've been stated over and over again in this thread and actually paraphrased from the first paragraph on my original email. It'd be cool if we could stop restating the obvious over and over again. So you did, sorry. Could someone give me an example of how we are supposed to improve the patch or commit message when one get a -1 with e.g. the question: Why do you use getattr(foo, bar, None)? By calling them out in the review or on irc, and explain to them when its appropriate to use a -1. I don't think its safe to assume that a significant number of people who do these -1s are read every thread on the ML. when the answer is Well, otherwise it will raise an error and the code will fail because the reviewer do not know how getattr() works. -- Julien Danjou // Free Software hacker // http://julien.danjou.info __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 2015-04-24 11:01:53 -0700 (-0700), Joe Gordon wrote: [...] in trying to figure out why that line is there I do a git blame only to see a useless commit message with me as the author. [...] Always wanted to travel back in time to try fighting a younger version of yourself? Software development is the career for you! [attributed to Elliot Loh] __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24 2015, Joe Gordon wrote: When I get a -1 on one of my patches with a question, I personally treat it as a short coming of the commit message. To often in the past I have looked at a file, and in trying to figure out why that line is there I do a git blame only to see a useless commit message with me as the author. That's a thing that I've been stated over and over again in this thread and actually paraphrased from the first paragraph on my original email. It'd be cool if we could stop restating the obvious over and over again. Could someone give me an example of how we are supposed to improve the patch or commit message when one get a -1 with e.g. the question: Why do you use getattr(foo, bar, None)? when the answer is Well, otherwise it will raise an error and the code will fail because the reviewer do not know how getattr() works. -- Julien Danjou // Free Software hacker // http://julien.danjou.info signature.asc Description: PGP signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
Excerpts from Jeremy Stanley's message of 2015-04-24 15:15:18 +: On 2015-04-24 10:23:35 -0400 (-0400), Doug Hellmann wrote: [...] 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. [...] Thinking about this though, for the benefit of non-fluent readers and cultures where rhetorical questions are a less common construct, it's probably better if we make a conscious effort to actually say what we mean and give directly actionable feedback when reviewing. Please add code comments here explaining the behavior in the case where Y happens, since it isn't well tested in our unit tests. (Or even better, please add tests!) A lot of the good questions I see in reviews where there's a -1 (and I'm frequently guilty of this too) could be much more effectively phrased as requests to improve code comments, use clearer syntax, add more detail in commit messages, or even better test the code being added/changed. That's a fair point. I tend to think of the questions as a way to start the discussion about the patch, rather than a subtle hint at what I think might be wrong. If that conversation reaches a point where more work is obviously needed, then I go ahead and make that request more plainly. Doug __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
+1 about Dan's comments. 1. We should not discourage all cases for -1 for questions. Because it often leads more discussion about the code issue, it is helpful for such case. Of course, it is diffcult to find a balance point, what can -1, what can 0. I don't think 0 in gerrit works well, because sometimes authors not care about that. 2. Also, for typos in comments and message, -1 makes sense too. Because 0 in gerrit means no need to improve the message and everything is good. It can lead many bad cases for cores, and non-cores. It means it doesn't matter if spell right or wrong. Best Wishes, Follow your heart. You are miracle! From: Dan Smith d...@danplanet.com To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.org Date: 04/24/2015 10:48 PM Subject:Re: [openstack-dev] Please stop reviewing code while asking questions 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. Right, and -1 makes the comment much more visible to both other cores and the reviewer. Questions which rightly point out something which would lead to what the OP considers a legit -1 can *easily* get missed in the wash of review comments on a bug. If you leave a -1 for a question and never come back to drop it when the answer is provided, then that's bad and you should stop doing that. However, I'm really concerned about the suggestion to not -1 for questions in general because of the visibility we lose. I also worry that more non-core people will feel even less likely to -1 a patch for something they feel is just their failing to understand, when in fact it's valuable feedback that the code is obscure. As a core, I don't exclude all reviews with a -1, and doing so is pretty dangerous behavior, IMHO. I'm not sure if the concern of -1s for questions is over dropping the review out of the hitlist for cores, or if it's about hurting the feelings of the submitter. I'm not in favor of discouraging -1s for either problem. --Dan __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 04/24/2015 07:26 AM, Salvatore Orlando wrote: If you think it might be beneficial to adjust tooling to that these contributions get counted this is fine by me. I just wanted to point out that I do not consider those contributions at all (and btw it would be at least more polite to put a +1 rather than a -1). If you're asking a question to elicit information, then it's quite possible you don't have enough information for a +1 yet. Chris __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 24 April 2015 at 16:50, Chris Friesen chris.frie...@windriver.com wrote: On 04/24/2015 07:26 AM, Salvatore Orlando wrote: If you think it might be beneficial to adjust tooling to that these contributions get counted this is fine by me. I just wanted to point out that I do not consider those contributions at all (and btw it would be at least more polite to put a +1 rather than a -1). If you're asking a question to elicit information, then it's quite possible you don't have enough information for a +1 yet. This makes sense in general. I was referring to the specific cases posed by Julien - curiosities, pedantry, or questions unrelated to the scope of the patch. Julien clarified that there actually questions which grant a -1, and surely never a +1. For instance the kind of what if questions listed by Doug. In this case it make sense for a reviewer to put a hold a patch while waiting for an answer. Chris __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 2015-04-24 10:23:35 -0400 (-0400), Doug Hellmann wrote: [...] 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. [...] Thinking about this though, for the benefit of non-fluent readers and cultures where rhetorical questions are a less common construct, it's probably better if we make a conscious effort to actually say what we mean and give directly actionable feedback when reviewing. Please add code comments here explaining the behavior in the case where Y happens, since it isn't well tested in our unit tests. (Or even better, please add tests!) A lot of the good questions I see in reviews where there's a -1 (and I'm frequently guilty of this too) could be much more effectively phrased as requests to improve code comments, use clearer syntax, add more detail in commit messages, or even better test the code being added/changed. -- Jeremy Stanley __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
And if anyone wants to take this path, I’d like to point you to two additional references. https://review.openstack.org/#/c/115778/ https://github.com/openstack/trove/commit/52b78a9d87351a542441a27396d3c7c38cc2d3ce Thanks, -amrith From: Brant Knudson [mailto:b...@acm.org] Sent: Friday, April 24, 2015 11:16 AM To: OpenStack Development Mailing List (not for usage questions) Subject: Re: [openstack-dev] Please stop reviewing code while asking questions On Fri, Apr 24, 2015 at 10:06 AM, Salvatore Orlando sorla...@nicira.commailto:sorla...@nicira.com wrote: On 24 April 2015 at 16:50, Chris Friesen chris.frie...@windriver.commailto:chris.frie...@windriver.com wrote: On 04/24/2015 07:26 AM, Salvatore Orlando wrote: If you think it might be beneficial to adjust tooling to that these contributions get counted this is fine by me. I just wanted to point out that I do not consider those contributions at all (and btw it would be at least more polite to put a +1 rather than a -1). If you're asking a question to elicit information, then it's quite possible you don't have enough information for a +1 yet. This makes sense in general. I was referring to the specific cases posed by Julien - curiosities, pedantry, or questions unrelated to the scope of the patch. Julien clarified that there actually questions which grant a -1, and surely never a +1. For instance the kind of what if questions listed by Doug. In this case it make sense for a reviewer to put a hold a patch while waiting for an answer. Would anybody be willing to codify this into a document that we can point offenders to so that we can get better review quality over time? Maybe http://docs.openstack.org/infra/manual/developers.html#peer-review (was https://wiki.openstack.org/wiki/ReviewChecklist ). (Just don't be surprised when some joker posts a question with a -1 on the review.) - Brant __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 04/24/2015 04:14 AM, Julien Danjou 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* Counterpoint: If you don't, you will not get a response from the author. Too much information, too much churn, but a -1 demands a r response, and 0 does not. Perhaps if we could separately indicate a question that needs to be answered from a do not merge message, this would changesomething like: Q: Is Foo the bar? sets the question flag. Me: No Foo is not the bar. Indicates that I think I have answered the question and clears the flag. Q then gets notification, and can say yes or no. If unanswered after a day/week, the Flag is automatically cleared. But...that is what -1 is for. It means: don't merge until my question is answered. If an author could mark that they feel they've addressed the -1 without a resubmission, it would take the stink off the -1. _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! __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24 2015, Doug Hellmann wrote: 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. Well I didn't say you weren't allowed to ask questions. I said you cannot ask a question because you don't know something _and_ set a score. The cases you describe matches my first paragraph which says: _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. And in the case you describe I think it's even better to gently ask for a test that proves that the case Y is not broken if the test does not exist yet. 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. What? Why do you say this now? If I knew years ago then I would have never have written a line of unit test! I would have replaced all of those tests with a simple Hey, this code totally works! in each commit message! ;-) Seriously, I think this is too kind/dangerous and that's it's just even better to ask for tests, full stop. 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. Yeah I don't think what you describe matches the *bad* behavior I described in my original email. Sorry. I can understand you wanted to appear as the bad guy I was yelling at, but that was not you after all! ;) -- Julien Danjou -- Free Software hacker -- http://julien.danjou.info signature.asc Description: PGP signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 04/24/2015 10:11 AM, Morgan Fainberg wrote: 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. +1 for giving a -1 :) If I don't understand the part of the code that is being patched, I don't review it. But if I do know that part of the code and I can't figure out what the thinking behind the patch is, I belive that a -1 is the appropriate response. I would certainly expect that for any unclear patches I submitted. The flip side of this, of course, is if I -1 something, I fully intend to follow up on the patch, and if it is clarified, remove the -1. The bigger problem is the hit-and-run approach of -1'ing a patch and then never returning. - -- - -- Ed Leafe -BEGIN PGP SIGNATURE- Version: GnuPG v2 Comment: GPGTools - https://gpgtools.org Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBCgAGBQJVOmE5AAoJEKMgtcocwZqLQXkQAKCaFJ0u4/RvPMPj5n1OCsoZ vx3lqGshJa2aHGhiX20e9EFIBDzSTPuM06ZJdsKWsRskdK3U/is2pHP7st/PrRUu okMUOodti8bA4ju2PNxXOKLioKakQghQf7I8E3b531m4Vx4+x7OBRHXYfe1YT8lL asOoba9GzkGoOqqGgfI4x2wRjU63kic3dqUTtEA1ResAnX5xZYxXWj0YHS3F6N12 kmLNBDbqB9BRvF9eBVOeIKqNRQ67/D0TmnJx4zze8lJnoI8Pi2BHmQIPcnS7tv1n jSgB6LUGh09tJ/jVyhHlFTZfnYaG4UDzOw1P7B/TNIv8yIpHZruii5NfYzGolbom 4/ZKwFMGHgvGEKoV9s/8E4TteebsQaYAYwwPrjvTdaJd67nflOJ8tdHWLhiNehLx urhPSyzAj5lwp51r2qTCEeRoaU7l5Pjl8Sq3LSpo3FX9aFb6tLrR9oSfju6gMeBC 7UiRAYo4uXLw2GHZ5/SmoECspG0WWOD+kPKayBykknX3d1mBXkmJxBfFAADPDWv3 ymahhN1DECiKwXn8lQn3YLGqduUxPJKMhjGZLh7iIVknYHDze2CTp88VM7kxO1w/ 9vyCcwvpnR7ahSE1lLkIVYHQ6HiyxD7jIP99tLx9Qfv3dKo4L0vtszcizAbJluHt izUfXSL4t0+oPRiKbcxY =eSbr -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
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. Right, and -1 makes the comment much more visible to both other cores and the reviewer. Questions which rightly point out something which would lead to what the OP considers a legit -1 can *easily* get missed in the wash of review comments on a bug. If you leave a -1 for a question and never come back to drop it when the answer is provided, then that's bad and you should stop doing that. However, I'm really concerned about the suggestion to not -1 for questions in general because of the visibility we lose. I also worry that more non-core people will feel even less likely to -1 a patch for something they feel is just their failing to understand, when in fact it's valuable feedback that the code is obscure. As a core, I don't exclude all reviews with a -1, and doing so is pretty dangerous behavior, IMHO. I'm not sure if the concern of -1s for questions is over dropping the review out of the hitlist for cores, or if it's about hurting the feelings of the submitter. I'm not in favor of discouraging -1s for either problem. --Dan __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Apr 24, 2015, at 06:27, Brant Knudson b...@acm.org wrote: On Fri, Apr 24, 2015 at 3:14 AM, Julien Danjou jul...@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-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24, 2015 at 10:06 AM, Salvatore Orlando sorla...@nicira.com wrote: On 24 April 2015 at 16:50, Chris Friesen chris.frie...@windriver.com wrote: On 04/24/2015 07:26 AM, Salvatore Orlando wrote: If you think it might be beneficial to adjust tooling to that these contributions get counted this is fine by me. I just wanted to point out that I do not consider those contributions at all (and btw it would be at least more polite to put a +1 rather than a -1). If you're asking a question to elicit information, then it's quite possible you don't have enough information for a +1 yet. This makes sense in general. I was referring to the specific cases posed by Julien - curiosities, pedantry, or questions unrelated to the scope of the patch. Julien clarified that there actually questions which grant a -1, and surely never a +1. For instance the kind of what if questions listed by Doug. In this case it make sense for a reviewer to put a hold a patch while waiting for an answer. Would anybody be willing to codify this into a document that we can point offenders to so that we can get better review quality over time? Maybe http://docs.openstack.org/infra/manual/developers.html#peer-review (was https://wiki.openstack.org/wiki/ReviewChecklist ). (Just don't be surprised when some joker posts a question with a -1 on the review.) - Brant __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 25 April 2015 at 01:13, Zane Bitter zbit...@redhat.com wrote: On 24/04/15 07:21, Amrith Kumar wrote: Julien, We had a similar discussion within Trove several months ago and agreed to a convention that if you have a question, that should not warrant a -1 unless, as you indicate there's a strong reason to believe that the code is wrong and the question is leading. +1. I'm kind of shocked that this even needed to be discussed, but well done :) We discussed this at a mid-cycle and agreed to put our conventions in CONTRIBUTING.rst[1]. We had a hypothesis about why +0 was rarely used (never conclusively proved). Our hypothesis was that since Stackalytics didn't count +0's it led to an increased propensity to -1 something. It would be wonderful if we could try the experiment of giving credit for 0's and seeing if it changes behavior. IIRC the problem here is the Gerrit API - it doesn't count +0 as a 'review', so they just don't show up in any automated tools. (This isn't easily solved either, even assuming you're willing to modify Gerrit.) The comments are definitely accessible over the new JSON API that gertty uses, so we can solve this now, however fugly it might look in reviewstats. Individualised closed-loop metrics *always* drive bad behaviour, because they're necessarily only a sample of the behaviour we care about and to the extent that sample is representative of the whole, it can only remain so in the open-loop case. So we can, and should, tweak metrics to reduce bad behaviour and encourage good behaviour, but we shouldn't kid ourselves that we can eliminate unintended consequences - we can only exchange them for _different_ unintended consequences. This is an open community, so we can't (and shouldn't want to) prevent people from publishing stats. The best case is that we use them only to inform us how we're doing in the aggregate, and discourage companies in particular from attaching individual incentives to game the metrics. Members of the TC, at least, (I don't know that there was ever an official edict or anything) have expressed this in the past, and I think it's one of those things that requires vigilance and periodic reminders. We could publish a document of common bad metrics and why we as a community reject them. That might give folk inside contributing companies some additional support in convincing metric-users not to look at some of the metrics out there. -Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 25 April 2015 at 05:43, Ben Nemec openst...@nemebean.com wrote: On 04/24/2015 07:11 AM, Russell Bryant wrote: On 04/24/2015 07:21 AM, Amrith Kumar wrote: We had a hypothesis about why +0 was rarely used (never conclusively proved). Our hypothesis was that since Stackalytics didn't count +0's it led to an increased propensity to -1 something. It would be wonderful if we could try the experiment of giving credit for 0's and seeing if it changes behavior. I think this makes a lot of sense. These stats really do drive behavior. I'd certainly be open to a patch to reviewstats [1] to count +0 comments and I think it would be good for stackalytics to consider the same. [1] http://git.openstack.org/cgit/openstack-infra/reviewstats I actually looked at doing this a while ago, but unfortunately the json data we were getting from Gerrit didn't include no scores. That may have been pre-Gerrit upgrade so it's possible it will have changed, but it's worth noting that we may just not have the ability to track this. I believe we'll need to switch to the REST JSON API, which gertty uses and gets comments - some nontrivial refactoring implied, and I shudder to think about the performance impact. I've wanted to rewrite reviewstats for a while... this might be the trigger... -Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 25 April 2015 at 00:11, Russell Bryant rbry...@redhat.com wrote: On 04/24/2015 07:21 AM, Amrith Kumar wrote: We had a hypothesis about why +0 was rarely used (never conclusively proved). Our hypothesis was that since Stackalytics didn't count +0's it led to an increased propensity to -1 something. It would be wonderful if we could try the experiment of giving credit for 0's and seeing if it changes behavior. I think this makes a lot of sense. These stats really do drive behavior. I'd certainly be open to a patch to reviewstats [1] to count +0 comments and I think it would be good for stackalytics to consider the same. [1] http://git.openstack.org/cgit/openstack-infra/reviewstats IIRC the limit was gerrit, not reviewstats, and we've not revisited since the gerrit upgrade. -Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 04/24/2015 09:27 AM, Brant Knudson wrote: On Fri, Apr 24, 2015 at 3:14 AM, Julien Danjou jul...@danjou.info mailto:jul...@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. This is absolutely correct, IMHO, and the reason why I always encourage people to ask questions about things that are unclear and point out that a code comment explaining the particular piece of code would be a welcome thing. Now, whether or not a -1 is warranted ... meh, usually not, but if the code really is too difficult to decipher or the comments are indecipherable, sometimes a -1 is appropriate. Best, -jay __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
Excerpts from Doug Hellmann's message of 2015-04-24 07:23:35 -0700: 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. Doug I think those questions fall into the bucket of leading, relevant questions. These are real -1 reviews. A change that doesn't have supporting evidence in the commit message, comments, and/or documentation, is often a ticking time-bomb that goes off when code is released into the wild, or sometimes much later when the what-if comes true. What I think Julien is frustrated by is not what happens if ... but I don't understand how this can work or can we really do X in library Y? type questions, which I've definitely seen too and have had reviews held up for months because the -1 drops it off many reviewers' radars. 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. Yes, I totally agree. Commit message is a good place to explain why the change is a) needed, and b) safe. Comments and docstrings are also a good place to explain (b), such as '''Mutates state in X No serialization is done in this method, ensure all callers lock X first''' But again, asking how does this new method ensure X mutations are atomic? is an awesome leading question, and nobody is frustrated about those. So, here's what I think is a decent rule, which we might want to put in our reviewer checklist. I've submitted a review for wording that I think might be appropriate. https://review.openstack.org/177364 __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
Excerpts from Amrith Kumar's message of 2015-04-24 15:02:01 +: 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. I've run into too many cases where a trivial change has an unintended consequence, so I suppose I'm more conservative with my code reviews. I don't use 0 very often at all, not because of any stats counting, but because I don't assume that conveys any information to the author or other reviewers. I vote the way I mean for my comments to be taken, so I use -1 to indicate that more work is needed, even if that work is just explaining something better or demonstrating that an edge case is going to be handled. 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. I'm not sure where all of the interest in stats counting comes from, but it's definitely not a motivation of my own behavior. Doug __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On 04/24/2015 07:11 AM, Russell Bryant wrote: On 04/24/2015 07:21 AM, Amrith Kumar wrote: We had a hypothesis about why +0 was rarely used (never conclusively proved). Our hypothesis was that since Stackalytics didn't count +0's it led to an increased propensity to -1 something. It would be wonderful if we could try the experiment of giving credit for 0's and seeing if it changes behavior. I think this makes a lot of sense. These stats really do drive behavior. I'd certainly be open to a patch to reviewstats [1] to count +0 comments and I think it would be good for stackalytics to consider the same. [1] http://git.openstack.org/cgit/openstack-infra/reviewstats I actually looked at doing this a while ago, but unfortunately the json data we were getting from Gerrit didn't include no scores. That may have been pre-Gerrit upgrade so it's possible it will have changed, but it's worth noting that we may just not have the ability to track this. -Ben __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24, 2015 at 10:31 AM, Doug Hellmann d...@doughellmann.com wrote: Excerpts from Amrith Kumar's message of 2015-04-24 15:02:01 +: 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. I've run into too many cases where a trivial change has an unintended consequence, so I suppose I'm more conservative with my code reviews. I don't use 0 very often at all, not because of any stats counting, but because I don't assume that conveys any information to the author or other reviewers. I vote the way I mean for my comments to be taken, so I use -1 to indicate that more work is needed, even if that work is just explaining something better or demonstrating that an edge case is going to be handled. When I get a -1 on one of my patches with a question, I personally treat it as a short coming of the commit message. To often in the past I have looked at a file, and in trying to figure out why that line is there I do a git blame only to see a useless commit message with me as the author. 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. I'm not sure where all of the interest in stats counting comes from, but it's definitely not a motivation of my own behavior. Doug __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
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:d...@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-requ...@lists.openstack.org?subject:unsubscribe | http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24 2015, Joe Gordon wrote: Hi Joe, By calling them out in the review or on irc, and explain to them when its appropriate to use a -1. I don't think its safe to assume that a significant number of people who do these -1s are read every thread on the ML. 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. Because that's a lot of time and energy I could also spend improving OpenStack, so please, let me hope that some of them read that thread. ;-) -- Julien Danjou # Free Software hacker # http://julien.danjou.info signature.asc Description: PGP signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
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. -- Ryan Brown / Software Engineer, Openstack / Red Hat, Inc. __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Please stop reviewing code while asking questions
On Fri, Apr 24, 2015 at 2:00 PM, Julien Danjou jul...@danjou.info 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. After a bit of due diligence to track down the reviewer, make a note, ignore that -1 and move on. To address the fact that the -1 causes the review to not appear in many people's dashboards, if that feels like an issue, do a trivial patchset to actually reset the -1 if no other changes are otherwise forthcoming. I've been guilty of doing this (forgetting about a -1 on a review) and like to think that I'd pass whatever bar was set for reviewers. Limiting the pool of reviewers really doesn't fix the problem and sets a bad tone for the project. See all of the discussions about core status and exclusivity, we don't need to inflict more of that on ourselves. dt -- Dean Troyer dtro...@gmail.com __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
