[Reviewers] dropped a couple changesets and flushed the queue

[Pierre-Yves David]

About that specific operation
-----------------------------

In the general case, I've been double checking with the original pusher
before dropping changeset. There is a handful of reason I made an
exception here:

  * First, there was 92 other patches waiting on top of the one dropped,
    some of them for over a week. To me, the situation was "pretty bad".
    Solving that was high priority,

  * Second, I was confident in my finding of significant undocumented
    backward compatibility breaking changes in both cases. BC breakage is
    something we take seriously in the mercurial project, so taking
    action before they get public make sense here.

  * Finally, at that specific time, I had a large enough chunk of time to
    take care of the dropping process. Our current tooling around
    dropping are not great and preserving 'accept' stamps is a bit
    tricky. In the past I ended up handling the dropping process for
    other reviewers because their were not too confident about it. In
    this case the 92 patches above it made the whole thing a bit scary.
    This is especially relevant since I was unsure when/if I would get
    time for that in a close future. (This timing assessment . I did got
    time to spend on Mercurial until today, almost one week later.)

So, it was thanksgiving week-end in the USA, the middle of the night for
Japan and a couple of day I had a chat with Yuya who said he was pretty
busy with other stuff at that time. IRC was dead quiet and nobody
reacted when I mention surgery on the pending stack. So after extensive
thinking, I decided to prioritize flushing the queue and made an
exception and dropped the couple of changeset that were in the way of
it. I wrote that email to the list to make sure other reviewers were
aware of it.


About rational for dropping Mads patch
--------------------------------------

My initial email amongst other thing contains two important data about
this.

1) a sort explanation of the reason why I dropped mads patch:

 >> Mads histedit series got dropped because it change existing behavior
 >> in way inconsistent with rebase and potentially harmful for user
 >> relying on backward compatibility.

2) an explicitly mention I had detailed explanation about these drops
coming on the public mailing list threads. Verbatim below:

 >> Long explanation for each drop are on the way on relevant thread. But
here is a short version:

I do not see these two elements taken in account in the main part of
your email replying to mine. Your objection to the drop of Mads patch
does not make any mention of elements in the "short explanation" and
later in that paragraph you requested a long version I already said to
be on the way.

I'm mentioning and highlighting this because this is becoming a 
recurring feeling that your reply does not takes in account important
elements of email you are replying to. If we leave this un-addressed
this will evolve in a major issue.


About Mads latency
------------------

I also noticed Mads as a large latency and I'm certainly willing for be
a bit more flexible with his patches. For example, I agree we could
take the code about bdiff with the code only benefiting the Cpython
version, it is unlikely he wrote the proper version soon and having it
somewhere is still a win.

However, there is a limit to the extend we can adapt our quality
standard. Undocumented backward compatibility changes are a place
were high latency means he is unlikely to made the adjustment and
follow-up needed to avoid shipping a regression.


Talking about Mads, I was visiting Unity last week and it seems likely
that Mads will be spending more time on upstream Mercurial in the near
future. (Another option seems that he just become a bartender for jazz
festival over frozen sea for the next couple of years).

On policy around dropping patch
-------------------------------

I agree that getting a green light from another reviewers before
dropping is a good thing. Ideally, you get the approval of reviewers who
initially push the change (and any additional reviewer who approved it).

However I trust other reviewer to be thoughtful and flexible on that.
If there is a legitimate reason to bypass it after careful
consideration, they should got for it.

Dropping a patches is not a big deal, it is easy to requeue it as draft
on the top of the start and resume the discussion. Right
now, the main pain around dropping is our tooling.

Cheers,

On 11/28/2016 01:41 AM, Augie Fackler wrote:
>
>> On Nov 27, 2016, at 7:32 PM, Pierre-Yves David
>> <pierre-yves.david at ens-lyon.org
>> <mailto:pierre-yves.david at ens-lyon.org>> wrote:
>>
>> Mads histedit series got dropped because it change existing behavior
>> in way inconsistent with rebase and potentially harmful for user
>> relying on backward compatibility.
>>
>>> 9375077f1ace histedit: make --keep work more like graft and use
>>> default phase for copies (madski)
>>
>
> It was not clear to me that there was consensus that the changes here
> were bad - I agree it would have been better as two patches, but other
> than that what am I missing that makes this worth dropping? History
> indicates it’ll be weeks-to-never that Mads comes back to it and it’s a
> clear defect fix on the whole, so I’d have liked it to be clearer on the
> public review thread what’s wrong here[0].
>
> Also, as a matter of policy, I think I’d us to (in general) get an ack
> from another reviewer before dropping changesets from committed on the
> public mailing list.



> Thanks!
> Augie
>
> 0: While I was drafting this, your response to the review thread came
> in. Your concerns have merit, but I still disagree with the order in
> which things were done.

-- 
Pierre-Yves David