[Reviewers] dropped a couple changesets and flushed the queue
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Wed Dec 28 16:17:51 UTC 2016
On 12/15/2016 11:24 PM, Augie Fackler wrote:
>
>> On Dec 15, 2016, at 3:40 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>
>>> I didn’t ever say it was unreasonable to have dropped these patches.
>>> I just haven’t personally observed any attempt to contact me before discarding a patch I’ve queued, so I wanted to bring it up as a potential policy point for discussion.
>>
>> I'm sorry but this is definitely not how I read your initial reply. I triple checked before writing my previous reply, and I doubled checked again before writing this one. To me the opening part (quoted below), just read like "I don't see why this was reasonable, especially in regard with Mads latency".
>>
>>>>> 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
>>
>> That part (above) is about 70% of the total message payload and came first. This two things alone probably guaranteed I received reply as "aggressive reaction to the drop" and not "just wanting to bring a policy point to discussion”.
>
> [...]
>
>> A short rephrasing of that feedback is "please try to balance positivish/negativish ration in your email to match your mind state).
>>
>> To make it very clear, I don't bring this up to paint Augie badly and I trust he meant good. I'm spending time explaining all this because that as also been a recurring issues in our community (from various people and affecting various people) and a significant causes for my burn out earlier this year.
>
> Something that would be productive for both of us would be to try and take the most charitable possible interpretation of any email -
Note that if a good share of my latency is just the size of my backlog
and my slow writing, a descent share of it comes from the fact I pretty
much always wait a fair amount of time to reply to something complex,
this both ensure I'm calm and that I had throughout thinking about the
topic. In addition, I lets the reply rest for a bit an revising
contentious area to make sure my thinking are not just based on a
misunderstanding. This does not yield perfect result, I think it
definitely help. I would advocate for you to do some of that.
(The proof-read phase is also the main reason such emails travels in group)
> I probably shouldn’t have sent my message without significant re-editing after your message on the review thread landed, and I’m sorry for that.
In this thread, I'm trying to bring attention to the fact that pretty
much all the data you wanted were -already- contained in my initial
email. Receiving the one on the review thread should not have bring you
much new things but details on already provided informations. I'll go
over this once more because I've not seen you acknowledge that yet :-/
In particular, that initial email is already telling you that a
detailled email is coming and that such discussion will be public. So
asking for a public discussion is not bringing anything new to the table.
(verbatim from my initial email below)
>>>>>> Long explanation for each drop are on the way on relevant
>>>>>> thread. But here is a short version:
In the same way, that initial email already contains new data about
these Mads patches. This new data points out that I found new and
serious things that triggers our policy regarding regression. I'm not
just acting to short-cut the ongoing discussion.
(verbatim from my initial email below)
>>>>>> Mads histedit series got dropped because it change existing
>>>>>> behavior in way inconsistent with rebase and potentially harmful
>>>>>> for user relying on backward compatibility.
Your email asked questions that were -already- answered in the initial
email you replied too. That was highly frustrating and one of the main
part of why I started that deeper discussion. And the fact I still have
to point it out at that stage of the discussion because you still don't
seems to acknowledge it is getting frustrating too.
>>>> 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.
>>>
>>> Note that I agree in this case the patches should have been discarded, but would have appreciated even a cursory attempt to contact me and ask before doing so. It’s a bad feeling for me to have my review work discarded without me having a chance understand why or even know it’s about to happen.
>>
>> Yes, I should have pinged you beforehand. I understand it could have been frustrating for you.
>
> It was deeply frustrating, because it felt like you were unilaterally discounting my review. I responded too quickly (and was unclear), and in so doing upset you (and made you feel undervalued). Sorry about that. :(
Apologies accepted, thanks. My general strategy to avoid getting into
frustration reinforcement loop is to avoid replying to email when I'm
frustrated. Worth case I wrote down a reply and wait to calm down to
re-read and maybe send it.
>> I'll make my best to put more effort into that if such situation arise.
>>
>> It is probably worth to note that your ban of all communications between us outside public channels, adds a significant barrier to communicating such notice. That was part of why I skipped prior-pinging and directly moved to drop + "notification email”
>
> I don’t want the “this patch might need to be dropped” discussion to be private …
[piece cut in half to clarify my answer].
I never said I wanted to full patch drop discussion in private. I'm
talking about a small ping like: "I've found a regression in Mads'
histedit patch, I'm about to drop it". Here is why being able to have
such private ping would be useful to me:
- I would not use public IRC for that, Mads was likely to be reading
the channel and such small ping was too short to make sure he would take
it well. Especially given the worn-out state I saw him 48h before. I
wanted his first contact to be the full explanation phrased with extra care.
- Writing that full email explanation in the thread is a process that
can me around one hour. I was unsure I would have time to get it in
proper state + proof-reading before going to bed, so I prioritized the
unblocking of the other patches. It turned out that I got the time and
energy to finalize such email while finalizing the drop. But that email
might has well got proof-read and sent on the next morning.
Here is some example of discussions I could have expected around that
private ping.
— marmoute: I've found a regression in Mads' histedit patch, I'm about
to drop it and unblock the 92 other patches above it.
— durin42: ha okay, thanks for letting me know, make sure to reply to
the public thread.
— marmoute: sure, will do.
Or:
— marmoute: I've found a regression in Mads' histedit patch, I'm
about to drop it and unblock the 92 other patches above it.
— durin42: Ho! I'm surprised do you have more details about that?
— marmoute: It is mostly about phases. The shipped version of histedit
rewrite secret changeset as secret. The new code promote them to draft.
That's the kind of regression that will bites people. (I'm also still
concerned about introducing an inconsistency with rebase but that's less
concerning.)
— durin42: okay, thanks for letting me know.
> … - I think it should be on the original review thread (if feasible), and at least on mercurial-devel at . That’s what I had in mind as far as a policy goes (so everything comes back to the same mailing list). Does that make sense?
It make so much sense that I've been having drop discussion in public
since forever and that my initial email explicitly said that would be
the case for that drop too. I don't see harm in formalizing the policy,
but to my knowledge all reviewers have been behaving well in that
regards so I've never been too concerns about this.
>> (Still would have been better to ping you beforehand instead of picking the easy option)
[I've split the rest of the question regarding the review tooling in a
separate thread because it end up being a large email on its own.]
Cheers,
--
Pierre-Yves David
More information about the Reviewers
mailing list