<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">100% agree here with Morgan. +1 is great, especially for projects with fewer regular reviewers where more eyes can help a lot.<br>
<br>
Best Regards, Erik Olof Gunnar Andersson<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>From:</b> Morgan Fainberg <morgan.fainberg@gmail.com> <br>
<b>Sent:</b> Tuesday, May 21, 2019 10:26 AM<br>
<b>To:</b> Jeremy Stanley <fungi@yuggoth.org><br>
<b>Cc:</b> openstack-discuss@lists.openstack.org<br>
<b>Subject:</b> Re: [dev][all] note to non-core reviewers in all projects<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal"><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.<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">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).<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">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<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Tue, May 21, 2019 at 9:42 AM Jeremy Stanley <<a href="mailto:fungi@yuggoth.org">fungi@yuggoth.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">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<o:p></o:p></p>
</blockquote>
</div>
</div>
</body>
</html>