[llvm-dev] Clarification on expectations of buildbot email notifications

[Philip Reames via llvm-dev]

+1 to Justin's comment

The only standard for revert should be: it's broken, and here's a 
reproducer.  Nothing else should matter.

... says the person w/a ton of internal regression testing which 
regularly finds crashes upstream, and is terrified of the implied effort 
of having to engage each author of a broken patch individually while 
being unable to ship or see what other breakage is coming in.  
Admittedly, self serving perspective.  :)

Philip

p.s. Speaking personally, I actually *prefer* having a patch of mine 
simply reverted w/o discussion when there's a problem.  This way there's 
no expectation of timely response on my side, and if I happen to be out 
of office, or on vacation, I can get to it when I get back and have 
time.  I do expect to be *informed* of the revert of course.

On 2/20/19 1:35 PM, Justin Bogner via llvm-dev wrote:
> Zachary Turner via llvm-dev <llvm-dev at lists.llvm.org> writes:
>> On Wed, Feb 20, 2019 at 9:13 AM Tom Stellard <tstellar at redhat.com> wrote:
>>> On 02/20/2019 07:39 AM, via llvm-dev wrote:
>>>> Reid said:
>>>>
>>>>> I don't think whether a buildbot sends email should have anything to do
>>>>> with whether we revert to green or not. Very often, developers commit
>>>>> patches that cause regressions not caught by our buildbots. If the
>>>>> regression is severe enough, then I think community members have the
>>>>> right, and perhaps responsibility, to revert the change that caused it.
>>>>> Our team maintains bots that build chrome with trunk versions of clang,
>>>>> and we identify many regressions this way and end up doing many reverts
>>>>> as a result. I think it's important to continue this practice so that
>>>>> we don't let multiple regressions pile up.
>>>> My team also has internal bots and we see breakages way more often than
>>>> we'd like.  We are a bit reluctant to just go revert something, though,
>>>> and typically try to engage the patch author first.
>>>>
>>>> Engaging the author has a couple of up-sides: it respects the author's
>>>> contribution and attention to the process; and once you've had to fix
>>>> a particular problem yourself (rather than someone else cleaning up
>>>> after your mess) you are less likely to repeat that mistake.
>>>>
>>> In my opinion engaging the author first is the best approach for internal
>>> bots, and I think this should be captured in some way in the developer
>>> guide.
>>>
>>> We don't want to send the message that is OK to revert patches at any
>>> time just because one of your internal tests failed.  In my experience
>>> most community members do engage with the author first, like Paul
>>> describes,
>>> so this isn't usually a problem, but new contributors may not be familiar
>>> with this precedent.
>>>
>>> -Tom
>> This is kind of what I was getting at with my original email, so thank you
>> for wording it better than I did.
>>
>> If we can agree that "contact the author first for internal bots" is better
>> than "revert automatically, even for internal bots" (which may not be the
>> case, I don't want to speak for others), then the problem becomes one of
>> defining what an "internal bot" is.  In my opinion, an internal bot it is
>> one that does does not satisfy both conditions: a) on the public waterfall,
>> b) automatically sends notifications, but perhaps not everyone agrees with
>> this definition.
> I would argue that "internal vs external" is the wrong division here.
> It does come up that internal bots or weird local configurations find
> significant problems in practice sometimes, and reverting to green can
> be completely reasonable for these cases. Obviously some discretion is
> necessary, but reverting more or less any change that causes issues
> severe enough to legitimately block you or seriously hamper your ability
> to notice further issues is fair game in my eyes.
>
> Like Reid says, the important part about reverting is the contract
> between the person doing the revert and the original committer. When a
> patch is reverted, the reverter has a responsibility for making sure the
> originator has the means to fix whatever problem they found. Any revert
> that isn't tied to a public bot absolutely must come with reasonable
> instructions to reproduce the problem, and the reverter needs to
> actively engage with the originator to get the patch back in in a form
> that doesn't hit the problem.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev