<div dir="ltr">I see another problem working on patchsets with lots of revisions and long-lived history, such as specs or a complex change. The opinions of several reviewers may be different. So first reviewer lefts a comment, the owner of the change amends the patch according to it. But after time and iterations pass (and because the history is very long or impossible to read) another reviewer who has not read the whole history, may come and put a -1 that contradicts the initial change. Things like that could be sorted if we keep shorter patchsets, and merge things that are production ready but not perfect, then we come up with follow-up patches to amend possible comments, or create enhancements.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 30, 2018 at 4:16 PM, Dmitry Tantsur <span dir="ltr"><<a href="mailto:dtantsur@redhat.com" target="_blank">dtantsur@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 05/30/2018 03:54 PM, Julia Kreger wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Tue, May 29, 2018 at 7:42 PM, Zane Bitter <<a href="mailto:zbitter@redhat.com" target="_blank">zbitter@redhat.com</a>> wrote:<br>
[trim]<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Since I am replying to this thread, Julia also mentioned the situation where<br>
two core reviewers are asking for opposite changes to a patch. It is never<br>
ever ever the contributor's responsibility to resolve a dispute between two<br>
core reviewers! If you see a core reviewer's advice on a patch and you want<br>
to give the opposite advice, by all means take it up immediately - with *the<br>
other core reviewer*. NOT the submitter. Preferably on IRC and not in the<br>
review. You work together every day, you can figure it out! A random<br>
contributor has no chance of parachuting into the middle of that dynamic and<br>
walking out unscathed, and they should never be asked to.<br>
<br>
</blockquote>
<br>
Absolutely agree! In the case that was in mind where it has happened<br>
to me personally, I think it was 10-15 revisions apart, so it becomes<br>
a little hard to identify when your starting to play the game of "make<br>
the cores happy to land it". It is not a fun game for the contributor.<br>
Truthfully it caused me to add the behavior of intentionally waiting<br>
longer between uploads of new revisions... which does not help at all.<br>
<br>
The other conundrum is when you disagree and the person has left a -1<br>
which blocks forward progress and any additional reviews since it gets<br>
viewed as "not ready", which makes it even harder and slower to build<br>
consensus. At some point you get into "Oh, what formatting can I<br>
change to clear that -1 because the person is not responding" mode.<br>
</blockquote>
<br></span>
This, by the way, is a much broader and interesting question. In case of a by-passer leaving a comment ("this link must be https") and disappearing, the PTL or any core can remove the reviewer from the review. What to do with a core leaving a comment or non-core leaving a potentially useful comment and going to PTO?<div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
At least beginning to shift the review culture should help many of these issues.<br>
<br>
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
<br>
</blockquote>
<br>
<br>
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><p style="font-weight:bold;margin:0;padding:0;font-size:14px;text-transform:uppercase;margin-bottom:0"><span>Yolanda</span> <span>Robla Mota</span></p>
<p style="font-weight:normal;font-size:10px;margin:0px 0px 4px;text-transform:uppercase"><span>Principal Software Engineer</span><span style="color:rgb(204,204,204)">, <span style="font-weight:normal;color:#aaa;margin:0">RHCE</span></span></p>
<p style="font-weight:normal;margin:0;font-size:10px;color:#999"><a style="color:#0088ce;font-size:10px;margin:0;text-decoration:none;font-family:'overpass',sans-serif" href="https://www.redhat.com" target="_blank">Red Hat <span><br><br></span></a></p>
<span style="font-size:10px;margin:0px;color:rgb(153,153,153)"><p style="font-size:10px;margin:0">C/Avellana 213</p></span>
<span><p style="font-size:10px;margin:0;color:#999">Urb Portugal</p></span>
<p style="font-weight:normal;margin:0px 0px 6px;font-size:10px;color:rgb(153,153,153)"><span style="margin:0px;padding:0px">
<a style="color:#0088ce;font-size:10px;margin:0;text-decoration:none;font-family:'overpass',sans-serif" href="mailto:yroblamo@redhat.com" target="_blank">yroblamo@redhat.com</a> </span>
<span href="tel:+34605641639">M: <a href="http://redhatemailsignature-marketing.itos.redhat.com/" style="color:#0088ce;font-size:11px;margin:0;text-decoration:none;font-family:'overpass',sans-serif" target="_blank">+34605641639</a> </span>
</p>
<a href="https://red.ht/sig" target="_blank"> <img src="https://www.redhat.com/profiles/rh/themes/redhatdotcom/img/logo-red-hat-black.png" width="90" height="auto"></a></div></div></div></div></div></div>
</div>