Open Stack

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.

On Wed, May 30, 2018 at 4:16 PM, Dmitry Tantsur <dtantsur at redhat.com> wrote:

> On 05/30/2018 03:54 PM, Julia Kreger wrote:
>
>> On Tue, May 29, 2018 at 7:42 PM, Zane Bitter <zbitter at redhat.com> wrote:
>> [trim]
>>
>>> Since I am replying to this thread, Julia also mentioned the situation
>>> where
>>> two core reviewers are asking for opposite changes to a patch. It is
>>> never
>>> ever ever the contributor's responsibility to resolve a dispute between
>>> two
>>> core reviewers! If you see a core reviewer's advice on a patch and you
>>> want
>>> to give the opposite advice, by all means take it up immediately - with
>>> *the
>>> other core reviewer*. NOT the submitter. Preferably on IRC and not in the
>>> review. You work together every day, you can figure it out! A random
>>> contributor has no chance of parachuting into the middle of that dynamic
>>> and
>>> walking out unscathed, and they should never be asked to.
>>>
>>>
>> Absolutely agree! In the case that was in mind where it has happened
>> to me personally, I think it was 10-15 revisions apart, so it becomes
>> a little hard to identify when your starting to play the game of "make
>> the cores happy to land it". It is not a fun game for the contributor.
>> Truthfully it caused me to add the behavior of intentionally waiting
>> longer between uploads of new revisions... which does not help at all.
>>
>> The other conundrum is when you disagree and the person has left a -1
>> which blocks forward progress and any additional reviews since it gets
>> viewed as "not ready", which makes it even harder and slower to build
>> consensus. At some point you get into "Oh, what formatting can I
>> change to clear that -1 because the person is not responding" mode.
>>
>
> 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?
>
>
>
>> At least beginning to shift the review culture should help many of these
>> issues.
>>
>> ____________________________________________________________
>> ______________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscrib
>> e
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>



-- 

Yolanda Robla Mota

Principal Software Engineer, RHCE

Red Hat

<https://www.redhat.com>

C/Avellana 213

Urb Portugal

yroblamo at redhat.com    M: +34605641639
<http://redhatemailsignature-marketing.itos.redhat.com/>
<https://red.ht/sig>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20180530/722830f0/attachment.html>