<div dir="ltr"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style="font-size:13px">This is the real reason people are hesitant to +2. If we strengthen<br></span><span style="font-size:13px">the meaning of +1 then people will just say "LGTM" or something else<br></span><span style="font-size:13px">to "approve" without taking responsibility. Posting +1 is a common<br></span><span style="font-size:13px">convention for voting and should not be given stronger meaning.</span></blockquote><div><br></div><div>How widespread is the non-binary and easier to misinterpret +1/+2 system? I had to explain it to someone recently and they looked at me really funny.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br style="font-size:13px"><span style="font-size:13px">The real problem we'd like to address is stagnation of MRs that<br></span><span style="font-size:13px">are ready but not merged. The syntax for approving is not very<br></span><span style="font-size:13px">important. </span></blockquote><div><br></div><div>Yes it won't solve the problem, but I think this cultural change will help a tad.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style="font-size:13px">We should identify reasons MRs stagnate and address<br></span><span style="font-size:13px">them directly:</span></blockquote><div> </div><div>Agree.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br style="font-size:13px"><span style="font-size:13px">* It may be a governance problem. No one has responsibility<br></span><span style="font-size:13px"> to ensure everything that is ready gets merged.</span></blockquote><div><div><br></div></div><div>Agree.</div><div><br></div><div><br></div><div>* I think the big thing is the time problem Will mentioned. Each merge request adds work to uninvolved developers. If our standard is that we only give approval after hours to days of thorough review in the general case then something is broken. If a cursory review says that probably isn't necessary than just give the approval and trust the author to follow through. If it looks like that is necessary than just day so rather than giving a whishy washy +1.</div><div><br></div><div>The +1=="LGTM but don't merge" adds nothing of value.</div><div><br></div><div>* I've also been convinced that the reviewer should not be responsible for monitoring the dashboards at all. Except in the case of newbie authors who don't know how to monitor the dashboards, we should trust each others capabilities and make the original author solely responsible. The important exception is when the original author is a newbie who probably isn't familiar with the dashboards.</div><div><br></div><div><br></div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature">David E DeMarle<br>Kitware, Inc.<br>R&D Engineer<br>21 Corporate Drive<br>Clifton Park, NY 12065-8662<br>Phone: 518-881-4909</div></div>
<br><div class="gmail_quote">On Thu, Jul 7, 2016 at 1:41 PM, Berk Geveci <span dir="ltr"><<a href="mailto:berk.geveci@kitware.com" target="_blank">berk.geveci@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">If there is new functionality, I do not approve without testing & documentation. If it's a bug fix, I may ask if there is a test that would fail without the fix.</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 7, 2016 at 1:38 PM, Will Schroeder <span dir="ltr"><<a href="mailto:will.schroeder@kitware.com" target="_blank">will.schroeder@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">:-) okay then in order to cross the road, I'll insist more strongly on good tests and documentation</div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 7, 2016 at 1:27 PM, Berk Geveci <span dir="ltr"><<a href="mailto:berk.geveci@kitware.com" target="_blank">berk.geveci@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span><span style="font-size:12.8px">> Maybe it's because I'm a chicken</span><br><div><span style="font-size:12.8px"><br></span></div></span><div><span style="font-size:12.8px">+1</span></div></div><div class="gmail_extra"><br><div class="gmail_quote"><span>On Thu, Jul 7, 2016 at 10:10 AM, Will Schroeder <span dir="ltr"><<a href="mailto:will.schroeder@kitware.com" target="_blank">will.schroeder@kitware.com</a>></span> wrote:<br></span><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">It's a time problem: I see chunks of code which at a superficial level seem fine and I'm willing to give it a +1. The problem with a +2 is that issuing such a score has implications that I've jumped into the code at a deep level and can vouch for it. This can occasionally mean hours or days of work hence the delay. Maybe it's because I'm a chicken and should have faith (or otherwise ensure) that there is enough testing to proof it out, but I feel reluctant in some cases to give a +2 without enough due diligence which takes time I often don't have. Besides more testing, one thing that might help would be more "documentation" providing me with the confidence that important issues have been considered and accounted for.....<div><br></div><div>Best,<br>W<br><div><br></div></div></div><div class="gmail_extra"><div><div><br><div class="gmail_quote">On Thu, Jul 7, 2016 at 9:56 AM, Brad King <span dir="ltr"><<a href="mailto:brad.king@kitware.com" target="_blank">brad.king@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 07/07/2016 08:56 AM, David E DeMarle wrote:<br>
> In either case both authors and reviewers are responsible for<br>
> watching the dashboards and addressing issues that come up afterward.<br>
<br>
This is the real reason people are hesitant to +2. If we strengthen<br>
the meaning of +1 then people will just say "LGTM" or something else<br>
to "approve" without taking responsibility. Posting +1 is a common<br>
convention for voting and should not be given stronger meaning.<br>
<br>
The real problem we'd like to address is stagnation of MRs that<br>
are ready but not merged. The syntax for approving is not very<br>
important. We should identify reasons MRs stagnate and address<br>
them directly:<br>
<br>
* It may be a governance problem. No one has responsibility<br>
to ensure everything that is ready gets merged.<br>
<br>
* It may be a workflow problem, like waiting for buildbot results<br>
to approve and then forgetting. The new workflow:buildbots<br>
label may help here.<br>
<br>
-Brad<br>
<br>
_______________________________________________<br>
Powered by <a href="http://www.kitware.com" rel="noreferrer" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" rel="noreferrer" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Search the list archives at: <a href="http://markmail.org/search/?q=vtk-developers" rel="noreferrer" target="_blank">http://markmail.org/search/?q=vtk-developers</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://public.kitware.com/mailman/listinfo/vtk-developers" rel="noreferrer" target="_blank">http://public.kitware.com/mailman/listinfo/vtk-developers</a><br>
<br>
</blockquote></div><br><br clear="all"><div><br></div></div></div><span><font color="#888888">-- <br><div data-smartmail="gmail_signature"><div dir="ltr"><div>William J. Schroeder, PhD<br>Kitware, Inc. - Building the World's Technical Computing Software<br>28 Corporate Drive<br>Clifton Park, NY 12065<br><a href="mailto:will.schroeder@kitware.com" target="_blank">will.schroeder@kitware.com</a><br><a href="http://www.kitware.com" target="_blank">http://www.kitware.com</a><br><a href="tel:%28518%29%20881-4902" value="+15188814902" target="_blank">(518) 881-4902</a></div></div></div>
</font></span></div>
<br>_______________________________________________<br>
Powered by <a href="http://www.kitware.com" rel="noreferrer" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" rel="noreferrer" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Search the list archives at: <a href="http://markmail.org/search/?q=vtk-developers" rel="noreferrer" target="_blank">http://markmail.org/search/?q=vtk-developers</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://public.kitware.com/mailman/listinfo/vtk-developers" rel="noreferrer" target="_blank">http://public.kitware.com/mailman/listinfo/vtk-developers</a><br>
<br>
<br></blockquote></div></div></div><br></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div data-smartmail="gmail_signature"><div dir="ltr"><div>William J. Schroeder, PhD<br>Kitware, Inc. - Building the World's Technical Computing Software<br>28 Corporate Drive<br>Clifton Park, NY 12065<br><a href="mailto:will.schroeder@kitware.com" target="_blank">will.schroeder@kitware.com</a><br><a href="http://www.kitware.com" target="_blank">http://www.kitware.com</a><br><a href="tel:%28518%29%20881-4902" value="+15188814902" target="_blank">(518) 881-4902</a></div></div></div>
</div>
</div></div></blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
Powered by <a href="http://www.kitware.com" rel="noreferrer" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" rel="noreferrer" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Search the list archives at: <a href="http://markmail.org/search/?q=vtk-developers" rel="noreferrer" target="_blank">http://markmail.org/search/?q=vtk-developers</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://public.kitware.com/mailman/listinfo/vtk-developers" rel="noreferrer" target="_blank">http://public.kitware.com/mailman/listinfo/vtk-developers</a><br>
<br>
<br></blockquote></div><br></div>