Open Stack

<Core / Leadership Hat> I 100% agree with Jeremy here. I also want to add
that a +1 without comment, even from a reviewer with no history, is an
indicator that the code was read. Everyone has to start somewhere. I am
100% ok with some bare/naked +1s as long as eventually those folks give
quality feedback (even as far as a -1 with a "hey this looks off" or a
no-score with a question). If it is a pattern of just +1 to their
friends/coworkers patches without review, I'll eventually ignore the +1s.

In short, I encourage +1 even without comment if it brings in new
contributors / reviewers. I hope that if there is a subsequent -1 on a
patch (or other comments) the reviewers giving the initial +1 will read the
comments and understand the reasoning for the -1 (-2, -1 workflow,
whatever).

I think what I am saying is: I hope we (as a community) don't chase folks
off because they +1'd without comment. Everyone starts somewhere

On Tue, May 21, 2019 at 9:42 AM Jeremy Stanley <fungi at yuggoth.org> wrote:

> On 2019-05-21 09:31:19 -0400 (-0400), Brian Rosmaita wrote:
> > A recent spate of +1 reviews with no comments on patches has put
> > me into grumpy-old-man mode.
>
> Welcome to the club, we probably have an extra lapel pin for you
> around here somewhere. ;)
>
> > A +1 with no comments is completely useless (unless you have a
> > review on a previous patch set with comments that have been
> > addressed by the author).
> [...]
>
> I think it's far more contextually nuanced than that. When I see a
> naked +1 comment on a change from a reviewer I recognize as having
> provided insightful feedback in the past, I tend to give that some
> weight. Ideally a +1 carries with it an implicit "this change makes
> sense to me, is a good idea for the project, and I don't see any
> obvious flaws in it." If I only ever see (or only mostly see) them
> from someone who I don't recall commenting usefully in recent
> history, I basically ignore it. The implication of the +1 for that
> reviewer is still the same, it's just that I don't have a lot of
> faith in their ability to judge the validity of the change if they
> haven't demonstrated that ability to me.
>
> > When you post your +1, please leave a comment explaining why you
> > approve, or at least what in particular you looked at in the patch
> > that gave you a favorable impression.  This whole open source
> > community thing is a collaborative effort, so please collaborate!
> > You comment does not have to be profound.  Even just saying that
> > you checked that the release note or docs on the patch rendered
> > correctly in HTML is very helpful.
> [,...]
>
> In an ideal world where we all have plenty of time to review changes
> and whatever else needs doing, this would be great. You know better
> than most, I suspect, what it's like to be a core reviewer on a
> project with a very high change-to-reviewer ratio. I don't
> personally think it's a healthy thing to hold reviews from newer or
> less frequent reviewers to a *higher* standard than we do for our
> core reviewers on projects. The goal is to improve our software, and
> to do that we need a constant injection of contributors with an
> understanding of that software and our processes and workflows. We
> should be giving them the courtesy and respect of assuming they have
> performed diligence in the course of a review, as encouragement to
> get more involved and feel a part of the project.
>
> As project leaders, it falls upon us to make the determination as to
> when feedback from a reviewer is pertinent and when it isn't, *even*
> if it requires us to keep a bit of context. But more importantly, we
> should be setting the example for how we'd like new folks to review
> changes, not simply telling them.
> --
> Jeremy Stanley
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20190521/41296bf2/attachment.html>