<div dir="ltr">On Wed, Jul 9, 2014 at 1:35 PM, Brady Eidson <span dir="ltr"><<a href="mailto:beidson@apple.com" target="_blank">beidson@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><blockquote type="cite"><div>On Jul 9, 2014, at 1:15 PM, Ryosuke Niwa <<a href="mailto:rniwa@webkit.org" target="_blank">rniwa@webkit.org</a>> wrote:</div>
<br><div><div dir="ltr">On Wed, Jul 9, 2014 at 1:08 PM, Brady Eidson <span dir="ltr"><<a href="mailto:beidson@apple.com" target="_blank">beidson@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div><blockquote type="cite"><div>On Jul 9, 2014, at 12:39 PM, Ryosuke Niwa <<a href="mailto:rniwa@webkit.org" target="_blank">rniwa@webkit.org</a>> wrote:</div>
<br><div><div dir="ltr">On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton <span dir="ltr"><<a href="mailto:timothy_horton@apple.com" target="_blank">timothy_horton@apple.com</a>></span> wrote:<br><div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><blockquote type="cite"><div>On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak <<a href="mailto:mjs@apple.com" target="_blank">mjs@apple.com</a>> wrote:</div>
<br><div><div style="word-wrap:break-word"><div><br></div>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.</div></div></blockquote>
<div><br></div></div><div>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.</div></div></div></blockquote><div><br></div><div>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.</div>
</div></div></div></div></blockquote><div><br></div></div></div>We already have an automated tool that quickly and easily notifies the author/reviewer, and that tool also happens to create the rollout patch.</div><div><br>
</div><div>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.</div><div><br></div><div>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.</div>
</div></blockquote><div><br></div><div>When the bug for a rollout is created, the original bug is automatically reopened.</div></div></div></div></div></blockquote><div><br></div></div>Which makes sense when a patch breaks something, whether the resolution is the author following up with a fix *or* the rollout committing.’</div>
<div><br></div><div>This is not a reason to avoid creating a rollout patch.</div><div><div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Also, the bot doesn't provide enough information as to what's breaking because it only takes a single line of description on IRC.</div>
</div></div></div></blockquote><div><br></div></div>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.</div></div></blockquote><div><br></div><div>
This is not a complaint about the tool. In practice, the bot can't figure out why a given patch needs to be rolled out. It's the responsibility of the person who is rolling out the patch to give necessary details.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div>It's crucial that whoever reverting a patch provide a detailed explanation on what build or test failed and provide a hyper link to <a href="http://build.webkit.org/" target="_blank">build.webkit.org</a>. Otherwise the original author and the reviewer may have no idea what went wrong.</div>
</div></div></div></blockquote></div>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.</div>
</div></blockquote><div><br></div><div>Not at all. The point is that the person who requested to rollout a patch should provide the detailed explanation as to why the patch has to be rolled out, or exactly what got broken by the patch.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>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.</div>
</div></blockquote><div><br></div><div>Giving a detailed description has already been a prerequisite to revert a patch. I don't see why we need to enhance the tool to continue doing what we have always done. If you want to enhance the tool to help this process, please go ahead but I'm not singing up to do that work.</div>
<div><br></div><div>- R. Niwa</div><div><br></div></div></div></div>