[webkit-dev] Windows (Temporary) Build Turmoil

[Brent Fulgham]

If you don’t care about building WebKit on Windows, you can stop reading now!

Still here?  Okay:

I’m attempting to make the Windows build rely less on the Cygwin tool chain. 
There are a number of reasons for this, but one of the major benefits I hope to 
derive is to reduce the amount of effort required to successfully build WebKit 
on Windows.

In service of that goal, I am actually making things slightly more complicated: 
you now need to install a Windows build of Perl and Python, You can use 
ActiveState’s builds (http://downloads.activestate.com), or the ‘official’ 
Windows ports of these products.

If you encounter any problems with this “dual Perl/dual Python” situation, 
please let me know and I’ll work to resolve whatever difficulty you encounter. 
However, based on my testing on our internal build machines I do not anticipate 
you will have any trouble.

Thanks,

-Brent___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wed, Jul 9, 2014 at 7:25 PM, Maciej Stachowiak  wrote:

>
> On Jul 9, 2014, at 4:45 PM, Ryosuke Niwa  wrote:
>
> On Wednesday, July 9, 2014, Brady Eidson  wrote:
>
>>
>> On Jul 9, 2014, at 4:15 PM, Ryosuke Niwa  wrote:
>>
>>  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.
>>
>>
>> I went through the first handful of emails on that thread.  The original
>> request that wasn't meeting a lot of opposition before I stopped digging
>> through the thread was:
>> "Please contact the author/reviewer and give them a reasonable amount of
>> time *before rolling out their patch*."
>>
>> I did not reach the message where the consensus was "contact the author
>> and reviewer manually, *do not use webkitbot*"
>>
>> I believe that using webkitbot:
>> 1 - Comments in a new bugzilla created specifically because there's an
>> issue
>> 2 - Comments in the original bugzilla notifying of an issue
>>
>
> It doesn't. The bot only files a new bug, make it a blocker of the
> original bug, and then reopen the bug.
>
> It doesn't copy over any comments made in the new bug for example.
>
>  Assuming my webkitbot command contains a description of the reason this
>> patch is suspect, including a URL to the failure, can you further explain
>> why using webkitbot is unreasonable?
>>
>
> I'm not saying that using webkitbot is unreasonable. I'm saying that the
> person trying to revert a patch should first inform the author/reviewer
> first BEFORE start reverting the patch.
>
> Since webkitbot doesn't automatically post the details as to what failures
> the patch caused, and one line description is almost never adequate (e.g.
> needs a hyperlink to buildbot page, test failure diff or error log, et
> c...), I don't see how using webkitbot in its current state could ever be
> adequate.
>
> Of course, I'm not saying that webkitbot could never be improved to do
> these things.
>
>
> What things need to be done in addition to using 'webkitbot rollout' to
> meet a sufficient standard of notification? I assume based on your comments
> that it should:
>
> (1) Add a comment to the original bug that caused the regression (maybe
> something like "this caused regression bug XXX" where XXX is the rollout
> bug).
> (2) Add links to diagnostic information about the problem (e.g. buildbot
> results page showing the failure, or website URL illustrating a
> regression). They should probably go in the bug reporting the regression,
> not the original bug.
>
> Anything else? It seems like (1) and (2) could be done manually while also
> using 'webkitbot rollout', and (1) could in principle be done by the bot.
> Would you object if someone used 'webkitbot rollout' and then did (1) and
> (2)?
>

It is my understanding that doing (1), (2), and 'webkit rollout' doesn't
contradict the previously reached consensus.  All I'm stating is doing (1)
and (2) before reverting the patch has been the consensus.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wed, Jul 9, 2014 at 5:05 PM, Brady Eidson  wrote:

> On Jul 9, 2014, at 4:45 PM, Ryosuke Niwa  wrote:
>
> Since webkitbot doesn't automatically post the details as to what failures
> the patch caused, and one line description is almost never adequate (e.g.
> needs a hyperlink to buildbot page, test failure diff or error log, et
> c...), I don't see how using webkitbot in its current state could ever be
> adequate.
>
>
> "This patch is a candidate for being rolled out because the build-bots
> have conclusively indicated it as breaking the build.  Please take a look
> within ~3 hours of this bug being filed or I will cq+ the rollout. The
> description of the build failure and details on why I think this patch
> broke things can be found here:
> http://build.webkit.org/details/for/the/breakage";
>
> That can be told to webkitbot today.  Is that not sufficient?  If not, why
> not?
>

While the information provided here is sufficient, that would create a bug
with the whole thing in its title.  And we don't normally put text such as
"I'm rolling out your patch in 3 hours if I don't hear back from you" in
the bug title itself.

Granted, this is something that could be improved.  If someone could make
that improvement, it would be incredibly useful.  Unfortunately, I don't
think we're living in such a world where we can rely solely on webkitbot to
do everything for us.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Maciej Stachowiak]

> On Jul 9, 2014, at 4:45 PM, Ryosuke Niwa  wrote:
> 
> On Wednesday, July 9, 2014, Brady Eidson  wrote:
> 
>> On Jul 9, 2014, at 4:15 PM, Ryosuke Niwa  wrote:
>> 
>>  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.
> 
> I went through the first handful of emails on that thread.  The original 
> request that wasn’t meeting a lot of opposition before I stopped digging 
> through the thread was:
> “Please contact the author/reviewer and give them a reasonable amount of time 
> before rolling out their patch.”
> 
> I did not reach the message where the consensus was “contact the author and 
> reviewer manually, do not use webkitbot”
> 
> I believe that using webkitbot:
> 1 - Comments in a new bugzilla created specifically because there’s an issue
> 2 - Comments in the original bugzilla notifying of an issue
> 
> It doesn't. The bot only files a new bug, make it a blocker of the original 
> bug, and then reopen the bug.
> 
> It doesn't copy over any comments made in the new bug for example.
> 
> Assuming my webkitbot command contains a description of the reason this patch 
> is suspect, including a URL to the failure, can you further explain why using 
> webkitbot is unreasonable?
> 
> I'm not saying that using webkitbot is unreasonable. I'm saying that the 
> person trying to revert a patch should first inform the author/reviewer first 
> BEFORE start reverting the patch.
> 
> Since webkitbot doesn't automatically post the details as to what failures 
> the patch caused, and one line description is almost never adequate (e.g. 
> needs a hyperlink to buildbot page, test failure diff or error log, et c...), 
> I don't see how using webkitbot in its current state could ever be adequate.
> 
> Of course, I'm not saying that webkitbot could never be improved to do these 
> things.

What things need to be done in addition to using 'webkitbot rollout’ to meet a 
sufficient standard of notification? I assume based on your comments that it 
should:

(1) Add a comment to the original bug that caused the regression (maybe 
something like “this caused regression bug XXX” where XXX is the rollout bug).
(2) Add links to diagnostic information about the problem (e.g. buildbot 
results page showing the failure, or website URL illustrating a regression). 
They should probably go in the bug reporting the regression, not the original 
bug.

Anything else? It seems like (1) and (2) could be done manually while also 
using ‘webkitbot rollout’, and (1) could in principle be done by the bot. Would 
you object if someone used ‘webkitbot rollout’ and then did (1) and (2)?

Is there anything it does that it should not do? I assume reopening the 
original bug before the rollout lands is a case of this but it’s not clear to 
me that this is a showstopper.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Brady Eidson]

> On Jul 9, 2014, at 4:45 PM, Ryosuke Niwa  wrote:
> 
> On Wednesday, July 9, 2014, Brady Eidson  wrote:
> 
>> On Jul 9, 2014, at 4:15 PM, Ryosuke Niwa  wrote:
>> 
>>  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.
> 
> I went through the first handful of emails on that thread.  The original 
> request that wasn’t meeting a lot of opposition before I stopped digging 
> through the thread was:
> “Please contact the author/reviewer and give them a reasonable amount of time 
> before rolling out their patch.”
> 
> I did not reach the message where the consensus was “contact the author and 
> reviewer manually, do not use webkitbot”
> 
> I believe that using webkitbot:
> 1 - Comments in a new bugzilla created specifically because there’s an issue
> 2 - Comments in the original bugzilla notifying of an issue
> 
> It doesn't. The bot only files a new bug, make it a blocker of the original 
> bug, and then reopen the bug.

I just tried this on IRC with a patch of mine.

webkitbot:
1 - Filed a new bug
2 - Included my rollout reason in the new bug
3 - Reopened the original bug
4 - Commented in the original bug “This is re-opened since this is blocked by 
bug x”
5 - Announced to the reviewer and author on IRC that this is taking place.

> It doesn't copy over any comments made in the new bug for example.

When was this mentioned as a requirement?

The requirement was that the patch author and reviewer get contacted.  They 
definitely are, as they have been pinged on IRC and directly CC’ed on the 
rollout bug.
Additionally, anybody CC’ed on the original bug also gets CC email notifying 
them of the bug they can follow for the rollout if they wish.

What is the purpose of copying comments between these two bugzillas?

> Assuming my webkitbot command contains a description of the reason this patch 
> is suspect, including a URL to the failure, can you further explain why using 
> webkitbot is unreasonable?
> 
> I'm not saying that using webkitbot is unreasonable. I'm saying that the 
> person trying to revert a patch should first inform the author/reviewer first 
> BEFORE start reverting the patch.

Right, and since I haven’t cq+’ed the rollout patch at this stage, I have 
contacted the author/reviewer BEFORE reverting the patch.

> Since webkitbot doesn't automatically post the details as to what failures 
> the patch caused, and one line description is almost never adequate (e.g. 
> needs a hyperlink to buildbot page, test failure diff or error log, et c...), 
> I don't see how using webkitbot in its current state could ever be adequate.

“This patch is a candidate for being rolled out because the build-bots have 
conclusively indicated it as breaking the build.  Please take a look within ~3 
hours of this bug being filed or I will cq+ the rollout. The description of the 
build failure and details on why I think this patch broke things can be found 
here:  http://build.webkit.org/details/for/the/breakage”

That can be told to webkitbot today.  Is that not sufficient?  If not, why not?

~Brady___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wednesday, July 9, 2014, Brady Eidson  wrote:

>
> On Jul 9, 2014, at 4:15 PM, Ryosuke Niwa  > wrote:
>
>  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.
>
>
> I went through the first handful of emails on that thread.  The original
> request that wasn't meeting a lot of opposition before I stopped digging
> through the thread was:
> "Please contact the author/reviewer and give them a reasonable amount of
> time *before rolling out their patch*."
>
> I did not reach the message where the consensus was "contact the author
> and reviewer manually, *do not use webkitbot*"
>
> I believe that using webkitbot:
> 1 - Comments in a new bugzilla created specifically because there's an
> issue
> 2 - Comments in the original bugzilla notifying of an issue
>

It doesn't. The bot only files a new bug, make it a blocker of the original
bug, and then reopen the bug.

It doesn't copy over any comments made in the new bug for example.

Assuming my webkitbot command contains a description of the reason this
> patch is suspect, including a URL to the failure, can you further explain
> why using webkitbot is unreasonable?
>

I'm not saying that using webkitbot is unreasonable. I'm saying that the
person trying to revert a patch should first inform the author/reviewer
first BEFORE start reverting the patch.

Since webkitbot doesn't automatically post the details as to what failures
the patch caused, and one line description is almost never adequate (e.g.
needs a hyperlink to buildbot page, test failure diff or error log, et
c...), I don't see how using webkitbot in its current state could ever be
adequate.

Of course, I'm not saying that webkitbot could never be improved to do
these things.

 - R. Niwa


-- 
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Brady Eidson]

> On Jul 9, 2014, at 4:15 PM, Ryosuke Niwa  wrote:
> 
>  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.

I went through the first handful of emails on that thread.  The original 
request that wasn’t meeting a lot of opposition before I stopped digging 
through the thread was:
“Please contact the author/reviewer and give them a reasonable amount of time 
before rolling out their patch.”

I did not reach the message where the consensus was “contact the author and 
reviewer manually, do not use webkitbot”

I believe that using webkitbot:
1 - Comments in a new bugzilla created specifically because there’s an issue
2 - Comments in the original bugzilla notifying of an issue
3 - Emails the author and reviewer by virtue of CC’ing them on the bugzilla(s)
4 - Does *not* roll out the patch.

Assuming my webkitbot command contains a description of the reason this patch 
is suspect, including a URL to the failure, can you further explain why using 
webkitbot is unreasonable?

Maybe that reasoning was reached in the thread you linked to and I didn’t find 
it yet.  I would appreciate it being restated here as the project has 
progressed quite a bit since Dec 2012.

Thanks,
~Brady

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wednesday, July 9, 2014, Ryosuke Niwa  wrote:

>
> On Wednesday, July 9, 2014, Brady Eidson  > wrote:
>
>>
>> On Jul 9, 2014, at 1:43 PM, Ryosuke Niwa  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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wednesday, July 9, 2014, Brady Eidson  wrote:

>
> On Jul 9, 2014, at 1:43 PM, Ryosuke Niwa  > 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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Brady Eidson]

> On Jul 9, 2014, at 1:43 PM, Ryosuke Niwa  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.

 Brady

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Dan Bernstein]

> On Jul 9, 2014, at 1:15 PM, Ryosuke Niwa  wrote:
> 
> When the bug for a rollout is created, the original bug is automatically 
> reopened.

This is a long-standing bug in the bot. The original bug should not be reopened 
until the change that fixed it is reverted.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wed, Jul 9, 2014 at 1:44 PM, Simon Fraser  wrote:

>
> On Jul 9, 2014, at 1:15 PM, Ryosuke Niwa  wrote:
>
> On Wed, Jul 9, 2014 at 1:08 PM, Brady Eidson  wrote:
>
>>
>> On Jul 9, 2014, at 12:39 PM, Ryosuke Niwa  wrote:
>>
>> On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton 
>> wrote:
>>
>>>
>>> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  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.
>
> Also, the bot doesn't provide enough information as to what's breaking
> because it only takes a single line of description on IRC.
>
> 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.
>
>
> I think the person who does the rollout should provide sufficient info in
> the rollout bug to justify the rollout. I would prefer this to new policy
> that requires emailing the committer and reviewer.
>

Well, providing details in the bug would essentially email the author & the
reviewer since they're auto cc'ed.  I agree commenting on the Bugzilla is
better so that others could follow.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Simon Fraser]

On Jul 9, 2014, at 1:15 PM, Ryosuke Niwa  wrote:

> On Wed, Jul 9, 2014 at 1:08 PM, Brady Eidson  wrote:
> 
>> On Jul 9, 2014, at 12:39 PM, Ryosuke Niwa  wrote:
>> 
>> On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton  wrote:
>> 
>>> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  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.
> 
> Also, the bot doesn't provide enough information as to what's breaking 
> because it only takes a single line of description on IRC.
> 
> 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.

I think the person who does the rollout should provide sufficient info in the 
rollout bug to justify the rollout. I would prefer this to new policy that 
requires emailing the committer and reviewer.

Simon


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wed, Jul 9, 2014 at 1:35 PM, Brady Eidson  wrote:

>
> On Jul 9, 2014, at 1:15 PM, Ryosuke Niwa  wrote:
>
> On Wed, Jul 9, 2014 at 1:08 PM, Brady Eidson  wrote:
>
>>
>> On Jul 9, 2014, at 12:39 PM, Ryosuke Niwa  wrote:
>>
>> On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton 
>> wrote:
>>
>>>
>>> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  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.
>

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.

> 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.

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.  If you want to enhance the tool to help this process,
please go ahead but I'm not singing up to do that work.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [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  wrote:
> 
> On Wed, Jul 9, 2014 at 1:08 PM, Brady Eidson  wrote:
> 
>> On Jul 9, 2014, at 12:39 PM, Ryosuke Niwa  wrote:
>> 
>> On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton  wrote:
>> 
>>> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  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


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wed, Jul 9, 2014 at 1:08 PM, Brady Eidson  wrote:

>
> On Jul 9, 2014, at 12:39 PM, Ryosuke Niwa  wrote:
>
> On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton 
> wrote:
>
>>
>> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  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.

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

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.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Brady Eidson]

> On Jul 9, 2014, at 12:39 PM, Ryosuke Niwa  wrote:
> 
> On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton  wrote:
> 
>> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  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.

Thanks,
~Brady

> 
> - R. Niwa
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Wed, Jul 9, 2014 at 12:35 PM, Tim Horton 
wrote:

>
> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  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.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Tim Horton]

> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  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.

>  - Maciej
> 
>> On Jul 9, 2014, at 11:40 AM, Ryosuke Niwa  wrote:
>> 
>> Yes.  The point is to do these things before telling webkitbot to rollout a 
>> patch.
>> 
>> - R. Niwa
>> 
>> 
>> On Wed, Jul 9, 2014 at 11:07 AM, Simon Fraser  wrote:
>> On Jul 9, 2014, at 7:43 AM, Ryosuke Niwa  wrote:
>> 
>>> Hi all,
>>> 
>>> This is a friendly remainder that you should
>>> comment on the associated bug
>>> email the patch author and the reviewer who reviewed the patch
>>> before rolling out / reverting a patch.
>> 
>> If you use webkitbot to roll out, is this necessary?
>> 
>> Simon
>> 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Maciej Stachowiak]

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.

 - Maciej

> On Jul 9, 2014, at 11:40 AM, Ryosuke Niwa  wrote:
> 
> Yes.  The point is to do these things before telling webkitbot to rollout a 
> patch.
> 
> - R. Niwa
> 
> 
> On Wed, Jul 9, 2014 at 11:07 AM, Simon Fraser  wrote:
> On Jul 9, 2014, at 7:43 AM, Ryosuke Niwa  wrote:
> 
>> Hi all,
>> 
>> This is a friendly remainder that you should
>> comment on the associated bug
>> email the patch author and the reviewer who reviewed the patch
>> before rolling out / reverting a patch.
> 
> If you use webkitbot to roll out, is this necessary?
> 
> Simon
> 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Simon Fraser]

With any waiting period before actually rolling out?

Simon

On Jul 9, 2014, at 11:40 AM, Ryosuke Niwa  wrote:

> Yes.  The point is to do these things before telling webkitbot to rollout a 
> patch.
> 
> - R. Niwa
> 
> 
> On Wed, Jul 9, 2014 at 11:07 AM, Simon Fraser  wrote:
> On Jul 9, 2014, at 7:43 AM, Ryosuke Niwa  wrote:
> 
>> Hi all,
>> 
>> This is a friendly remainder that you should
>> comment on the associated bug
>> email the patch author and the reviewer who reviewed the patch
>> before rolling out / reverting a patch.
> 
> If you use webkitbot to roll out, is this necessary?
> 
> Simon
> 
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

Yes.  The point is to do these things before telling webkitbot to rollout a
patch.

- R. Niwa


On Wed, Jul 9, 2014 at 11:07 AM, Simon Fraser 
wrote:

> On Jul 9, 2014, at 7:43 AM, Ryosuke Niwa  wrote:
>
> Hi all,
>
> This is a friendly remainder that you should
>
>- comment on the associated bug
>- email the patch author and the reviewer who reviewed the patch
>
> before rolling out / reverting a patch.
>
>
> If you use webkitbot to roll out, is this necessary?
>
> Simon
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Simon Fraser]

On Jul 9, 2014, at 7:43 AM, Ryosuke Niwa  wrote:

> Hi all,
> 
> This is a friendly remainder that you should
> comment on the associated bug
> email the patch author and the reviewer who reviewed the patch
> before rolling out / reverting a patch.

If you use webkitbot to roll out, is this necessary?

Simon

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

Hi all,

This is a friendly remainder that you should

   - comment on the associated bug
   - email the patch author and the reviewer who reviewed the patch

before rolling out / reverting a patch.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Inactive Committers and Reviewers

[Ryosuke Niwa]

Hello WebKittens,

WebKit reviewers recently had a discussion about the large number of
inactive committers and reviewers left after the Blink fork, and we've come
to introduce a new policy to consider committers and reviewers who have not
contributed to the project over one year "*inactive*".  In addition, any
subversion account that hasn't been used to commit a code change to
svn.webkit.org over one year is subject to the deactivation. [1]

The policy change has been enacted as of r170904
 which added the following section to the
WebKit Committers and Reviewer Policy
.


*Inactive Committer or Reviewer Status*

A WebKit Committer or Reviewer that has not been active in the project for
over a year is considered inactive. Activity for this purpose is defined as
landing at least one patch in the past year. Reviewers who have reviewed a
patch in the past year will also be considered active.

Inactive Committers can regain Active Committer status by landing (via the
Commit Queue) a non-trivial patch and asking on webkit-reviewers for a
return to Active status.

Inactive Reviewers need to show that they are making an effort to get
 familiar with the changes that have happened in the project since they
were
last active by landing at least 3 non-trivial patches. Once they have
landed the patches, they need to send an email requesting reactivation to
webkit-reviewers. This request needs the support of 2 Active Reviewers to
be granted.

Note that regardless of a Committer or Reviewer's activity status, any
subversion account that has not been used in the past year will be
deactivated for security purposes. For example, a Reviewer that has
reviewed a patch in the past year but has not committed may have their
subversion account deactivated. To reactivate a deactivated subversion
account, an Active Committer or Active Reviewer can send an email to
webkit-reviewers requesting it.


- R. Niwa

[1] For the initial mass deactivation, I will send an email to each address
associated with the subversion account and give the account owner an option
to keep it active.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev