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

[Ryosuke Niwa]

On Wednesday, July 9, 2014, Brady Eidson <beidson at apple.com> wrote:

>
> On Jul 9, 2014, at 1:43 PM, Ryosuke Niwa <rniwa at webkit.org
> <javascript:_e(%7B%7D,'cvml','rniwa at webkit.org');>> wrote:
>
>
> 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.
>>
>
> 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.
>
>
> Of course the bot can't know, and of course it's the rollout'er's
> responsibility.
>
> I believe the thing that has drawn this thread out was the request to "do
> this work manually before using the tool"
>
> But I find the request to "do this manually instead of using the tool"
> bizarre because:
> 1 - The tool objectively meets most of the requirements, except for
> forcing a detailed description and URL to the failure.
> 2 - The tool objectively meets all of the requirements if the person using
> it provides the necessary data to the tool.
> 3 - You requested that creating the rollout patch should *not* be done,
> even though nobody presented a reason why the mere existence of the rollout
> patch is a problem.
> 4 - Relying on tools for common processes is a *good* thing.
>
>  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.
>>
>
> 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.
>
>
> This can be done by manually looking up email addresses, emailing people,
> logging in to bugzilla, and typing a comment; Like you requested.
>
> Or this can be done by using the tool we already have, but being aware to
> give the full context and a URL to breakage.
>
> 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.
>>
>
> 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.
>
>
> I don't see the *need* either, because it already supports everything
> required.
>
> If you want to enhance the tool to help this process, please go ahead but
> I'm not singing up to do that work.
>
>
> I don't expect you to.  I'm just trying to make it clear that I'm not
> going to start performing a checklist of manual work instead as originally
> requested; I intend to keep using the tool, but being more aware of giving
> the additional context.
>

 Again, im not requesting anything new here. The consensus on webkit-dev
has been to ping the author/reviewer on IRC or via email and comment in the
original bug PRIOR to using webkitbot to start reverting the patch.

- R. Niwa


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