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

[via llvm-dev]

> -----Original Message-----
> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
> Philip Reames via llvm-dev
> Sent: Wednesday, February 20, 2019 11:26 PM
> To: Justin Bogner; Zachary Turner via llvm-dev
> Subject: Re: [llvm-dev] Clarification on expectations of buildbot email
> notifications
> 
> +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.  :)

(I haven't noticed you doing tons of reverts either, but maybe I just
haven't been looking for them.)

This is why my crew has a staging branch for merging in upstream stuff,
in proper Theory of Constraints "buffer" style.  Between upstream breaks
and merge issues, it's actually kind of rare to have a clean automated
merge onto our master branch.  We usually end up looking for a "mostly
harmless" merge point and manually bringing it in, then fix up the issues 
later, just so we can stay sort-of caught up.

But, it does give us some breathing room to engage authors and give them
a reasonable period to fix things up themselves, before it materially
affects our actual work branches.
--paulr

> 
> 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev