[webkit-dev] Comment on the bug & email author/reviewer before reverting a patch

[Brady Eidson]

> On Jul 9, 2014, at 1:15 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> 
> On Wed, Jul 9, 2014 at 1:08 PM, Brady Eidson <beidson at apple.com> wrote:
> 
>> On Jul 9, 2014, at 12:39 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>> 
>> On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton <timothy_horton at apple.com> wrote:
>> 
>>> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>>> 
>>> 
>>> Could we teach webkitbot to do an appropriate notification with a waiting period? Either as part of rollout or add a new command to do it.
>> 
>> It already does. The “waiting period” is defined by when the person who asked for the rollout sets the cq+ bit on the rollout patch.
>> 
>> I don't think creating a rollout patch should be the standard method of notifying the author/reviewer.  We should be informing the author/reviewer ahead of the time.
> 
> We already have an automated tool that quickly and easily notifies the author/reviewer, and that tool also happens to create the rollout patch.
> 
> As Tim points out, the rollout patch is never landed unless a reviewer (usually the person who created the rollout patch) sets the cq+ bit on it.
> 
> I don’t see what negative effect the mere existence of the rollout patch has, or why we should codify into the process that a rollout patch is *not* created when notifying the author/reviewer.
> 
> When the bug for a rollout is created, the original bug is automatically reopened.

Which makes sense when a patch breaks something, whether the resolution is the author following up with a fix *or* the rollout committing.’

This is not a reason to avoid creating a rollout patch.

> Also, the bot doesn't provide enough information as to what's breaking because it only takes a single line of description on IRC.

This seems like a complaint you have with the tool that can be fixed.

This is not a reason to avoid creating a rollout patch.

> It's crucial that whoever reverting a patch provide a detailed explanation on what build or test failed and provide a hyper link to build.webkit.org.  Otherwise the original author and the reviewer may have no idea what went wrong.

This statement seems at odds with how webkitbot (or an earlier form thereof) has been used countless times, since it has been reverting patches with only 1-line explanations for years without an uproar.

If the premise of this thread is “don’t rely on the tool we already have, and instead please manually look up email addresses and/or go to bugzilla to manually comment yourself”, then I disagree.

If the premise of this email thread is “please provide a detailed description of why a patch is a candidate to be rolled out, including a link to the build/test failures”, then I wholeheartedly agree that webkitbot should be enhanced to allow and encourage this.

~Brady


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-dev/attachments/20140709/3f1991da/attachment.html>