[llvm-dev] Clarification on expectations of buildbot email notifications (original) (raw)

Philip Reames via llvm-dev llvm-dev at lists.llvm.org
Wed Feb 20 20:26:19 PST 2019

+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

More information about the llvm-dev mailing list