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

[Ryosuke Niwa]

On Wednesday, July 9, 2014, Ryosuke Niwa <rniwa at webkit.org> wrote:

>
> On Wednesday, July 9, 2014, Brady Eidson <beidson at apple.com
> <javascript:_e(%7B%7D,'cvml','beidson at apple.com');>> wrote:
>
>>
>> On Jul 9, 2014, at 1:43 PM, Ryosuke Niwa <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.
>

For example, the following archive contains two threads on this topic:
https://lists.webkit.org/pipermail/webkit-dev/2012-December/thread.html#23142

- R. Niwa


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