Markus Zoeller <mzoel...@de.ibm.com> wrote:
From: Masayuki Igawa <masayuki.ig...@gmail.com>
To: "OpenStack Development Mailing List (not for usage questions)"
<openstack-dev@lists.openstack.org>
Date: 04/11/2016 03:20 AM
Subject: Re: [openstack-dev] [all][stackalytics] Gaming the Stackalytics
stats
2016-04-11 9:46 GMT+09:00 Matt Riedemann <mrie...@linux.vnet.ibm.com>:
On 4/10/2016 6:37 PM, Clint Byrum wrote:
Excerpts from Matt Riedemann's message of 2016-04-09 06:42:54 -0700:
There is also disincentive in +1ing a change that you don't
understand
and is wrong and then a core comes along and -1s it (you get dinged
for
the disagreement). And there is disincentive in -1ing a change for
the
wrong reasons (silly nits or asking questions for understanding). I
ask
a lot of questions in a lot of changes and I don't vote on those
because
it would be inappropriate.
Why is disagreement a negative thing? IMO, reviewers who agree too
much
are just part of the echo chamber.
__________________________________________________________________________
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
I'm not saying disagreement is a negative thing, I was saying there
are
times when I've seen people -1 for crazy nits, e.g. there should be a
blank
line between the bug ref and change-id in the commit message, or for
asking
questions for understanding (which, btw, I'm fine with -1 for 'add a
comment
because this is complicated and I didn't get it at first'). And I'm
also not
crazy about piling on or agreeing with everything either. My point is
I
think it's appropriate in a lot of cases to just not vote but still
comment.
I think we have some/many implicit rules for our review. There's a
document[1] for review
but it doesn't mention crazy nits. So should we add what we don't want
to see people -1 for?
[1] http://docs.openstack.org/infra/manual/developers.html#peer-review
My basic rule of thumb for voting is:
vote | translates to
---------------------
-1 | "I understand the code and your change and I'd rather not
| maintain it. My reasons are [...] and suggestions are [...]."
+1 | "I understand the code and your change. It improves the
project and I'd maintain it."
0 | "I don't get the code or your change. My questions are [...]."
If it already has at least one +2, I (usually) ignore it. The change
already has attention from the cores, it is unlikely that I can add
more valuable feedback to that change.
Oh. I think that’s not an ideal approach. As a core, I often times see
valuable and reasonable comments that were not pointed out by existing
cores who already voted with +2, from folks with no core status.
As a core for one of projects, I appreciate *a lot* when people do deep
reviews for patches, or just ask questions, even if not leaving a vote.
Cores are often dragged into multiple directions, and so can miss a legit
bug, while some other contributors can pull their attention to a problem.
So please, please, please comment on patches you review. If you have a
question, please ask it. You will learn something, and maybe the code will
become cleaner or at least easier to read [f.e. because authors will add
some comments to their patches to avoid later confusion and questions.]
Don’t assume cores are gods and spot every bug. Cores also do silly things.
Ihar
__________________________________________________________________________
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
