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