via llvm-dev
2019-Feb-20 15:39 UTC
[llvm-dev] Clarification on expectations of buildbot email notifications
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.> I think what's important, and what we should, after this discussion > concludes, put in the developer policy, is that the person doing the > revert has the responsibility to do their best to help the patch author > reproduce the problem or at least understand the bug. > > This can take many forms. They can link directly to an LLVM buildbot, > which should be self-explanatory as far as reproduction goes. It can be > an unreduced crash report. If they're nice, they can use CReduce to make > it smaller. But, a reverter can't just say "Revert rNNN, breaks > $RANDOM_PROJECT on x86_64-linux-gu". If they add, "reduction forthcoming" > and they deliver on that promise, I think we should support that. > > In other words, the bar to revert should be low, so we can do it fast > and save downstream consumers time and effort. If someone isn't making > a good faith effort to follow up after a revert, then authors have a > right to push back.We have been on the wrong side of a revert where it was "this broke us" and then nothing. I was inclined to just re-apply the patch, but that's my "Mr Grumpy" avatar talking. How do we address failure to conform to the community norms?> I agree with Paul that we should remove the text about checking nightly > builders. That suggestion seems a bit dated.That was Tom Stellard, not me, but I agree with him. --paulr
Tom Stellard via llvm-dev
2019-Feb-20 17:13 UTC
[llvm-dev] Clarification on expectations of buildbot email notifications
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>> I think what's important, and what we should, after this discussion >> concludes, put in the developer policy, is that the person doing the >> revert has the responsibility to do their best to help the patch author >> reproduce the problem or at least understand the bug. >> >> This can take many forms. They can link directly to an LLVM buildbot, >> which should be self-explanatory as far as reproduction goes. It can be >> an unreduced crash report. If they're nice, they can use CReduce to make >> it smaller. But, a reverter can't just say "Revert rNNN, breaks >> $RANDOM_PROJECT on x86_64-linux-gu". If they add, "reduction forthcoming" >> and they deliver on that promise, I think we should support that. >> >> In other words, the bar to revert should be low, so we can do it fast >> and save downstream consumers time and effort. If someone isn't making >> a good faith effort to follow up after a revert, then authors have a >> right to push back. > > We have been on the wrong side of a revert where it was "this broke us" > and then nothing. I was inclined to just re-apply the patch, but that's > my "Mr Grumpy" avatar talking. How do we address failure to conform to the > community norms? > >> I agree with Paul that we should remove the text about checking nightly >> builders. That suggestion seems a bit dated. > > That was Tom Stellard, not me, but I agree with him. > --paulr > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >
Zachary Turner via llvm-dev
2019-Feb-20 17:36 UTC
[llvm-dev] Clarification on expectations of buildbot email notifications
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. 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 > > >> I think what's important, and what we should, after this discussion > >> concludes, put in the developer policy, is that the person doing the > >> revert has the responsibility to do their best to help the patch author > >> reproduce the problem or at least understand the bug. > >> > >> This can take many forms. They can link directly to an LLVM buildbot, > >> which should be self-explanatory as far as reproduction goes. It can be > >> an unreduced crash report. If they're nice, they can use CReduce to make > >> it smaller. But, a reverter can't just say "Revert rNNN, breaks > >> $RANDOM_PROJECT on x86_64-linux-gu". If they add, "reduction > forthcoming" > >> and they deliver on that promise, I think we should support that. > >> > >> In other words, the bar to revert should be low, so we can do it fast > >> and save downstream consumers time and effort. If someone isn't making > >> a good faith effort to follow up after a revert, then authors have a > >> right to push back. > > > > We have been on the wrong side of a revert where it was "this broke us" > > and then nothing. I was inclined to just re-apply the patch, but that's > > my "Mr Grumpy" avatar talking. How do we address failure to conform to > the > > community norms? > > > >> I agree with Paul that we should remove the text about checking nightly > >> builders. That suggestion seems a bit dated. > > > > That was Tom Stellard, not me, but I agree with him. > > --paulr > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190220/d17b21cf/attachment.html>