Re: [webkit-dev] Behaviour of CapsLock in WebKit/Mac

2012-12-11 Thread Wez
This seems to be Mac-specific in Chromium browsers.

Wez


On Tuesday, 11 December 2012, Elliott Sprehn wrote:

> Does this reproduce on every platform? If it's a OS X issue then it should
> work on Windows or Linux I'd hope.
>
> - E
>
>
> On Tue, Dec 11, 2012 at 9:32 PM, Mark Rowe  'cvml', 'mr...@apple.com');>
> > wrote:
>
>>
>> On 2012-12-11, at 21:24, Wez > 'cvml', 'w...@chromium.org');>> wrote:
>>
>> > Hi all,
>> >
>> > There's a bug reported against Chromium (crbug.com/144757) for the
>> CapsLock key generating only a keydown when first pressed and released, and
>> a keyup when next pressed and released, i.e. the keydown & keyup events
>> correspond with the caps lock-state being toggled, rather than with the key
>> itself being pressed or released.
>> >
>> > The same issue reproduces against Safari on Mac.  Is this a by-design
>> behaviour of WebKit?
>>
>> It’s a known issue ().
>>
>> - Mark
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org > 'webkit-dev@lists.webkit.org');>
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Behaviour of CapsLock in WebKit/Mac

2012-12-11 Thread Elliott Sprehn
Does this reproduce on every platform? If it's a OS X issue then it should
work on Windows or Linux I'd hope.

- E


On Tue, Dec 11, 2012 at 9:32 PM, Mark Rowe  wrote:

>
> On 2012-12-11, at 21:24, Wez  wrote:
>
> > Hi all,
> >
> > There's a bug reported against Chromium (crbug.com/144757) for the
> CapsLock key generating only a keydown when first pressed and released, and
> a keyup when next pressed and released, i.e. the keydown & keyup events
> correspond with the caps lock-state being toggled, rather than with the key
> itself being pressed or released.
> >
> > The same issue reproduces against Safari on Mac.  Is this a by-design
> behaviour of WebKit?
>
> It’s a known issue ().
>
> - Mark
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Behaviour of CapsLock in WebKit/Mac

2012-12-11 Thread Mark Rowe

On 2012-12-11, at 21:24, Wez  wrote:

> Hi all,
> 
> There's a bug reported against Chromium (crbug.com/144757) for the CapsLock 
> key generating only a keydown when first pressed and released, and a keyup 
> when next pressed and released, i.e. the keydown & keyup events correspond 
> with the caps lock-state being toggled, rather than with the key itself being 
> pressed or released.
> 
> The same issue reproduces against Safari on Mac.  Is this a by-design 
> behaviour of WebKit?

It’s a known issue ().

- Mark

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


[webkit-dev] Behaviour of CapsLock in WebKit/Mac

2012-12-11 Thread Wez
Hi all,

There's a bug reported against Chromium (crbug.com/144757) for the CapsLock
key generating only a keydown when first pressed and released, and a keyup
when next pressed and released, i.e. the keydown & keyup events correspond
with the caps lock-state being toggled, rather than with the key itself
being pressed or released.

The same issue reproduces against Safari on Mac.  Is this a by-design
behaviour of WebKit?

Thanks,

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


Re: [webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects

2012-12-11 Thread Ryosuke Niwa
On Tue, Dec 11, 2012 at 8:44 PM, David Levin  wrote:

> Was this an isolated incident then?
>

Well, it was done by a Chromium contributor but I’ve seen it happening
every now and then in the past though I’ve lost those references. I’ve
given Darin (fishd) more details if you’re interested in helping us out :)

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


Re: [webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects

2012-12-11 Thread David Levin
Was this an isolated incident then?


On Tue, Dec 11, 2012 at 8:39 PM, Ryosuke Niwa  wrote:

> On Tue, Dec 11, 2012 at 1:44 PM, Ryosuke Niwa  wrote:
>
>> First off, I don't think we should be rolling out patches based solely on
>> a downstream test unless there is a clear evidence that the failure is a
>> real regression in WebKit that affects more than just the said downstream
>> project.
>>
>
> To understand this perspective, imagine that I've created a new app called
> SuperWebKitApp that's built on Chromium, EFL, GTK+, Qt, etc... ports in my
> spare time, and started rolling out all Chromium, EFL, GTK+, Qt, etc...
> patches that break automated tests in SuperWebKitApp, just giving
> hyperlinks to my buildbots. That's pretty upsetting, isn't it?
>
> - R. Niwa
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects

2012-12-11 Thread Ryosuke Niwa
On Tue, Dec 11, 2012 at 1:44 PM, Ryosuke Niwa  wrote:

> First off, I don't think we should be rolling out patches based solely on
> a downstream test unless there is a clear evidence that the failure is a
> real regression in WebKit that affects more than just the said downstream
> project.
>

To understand this perspective, imagine that I've created a new app called
SuperWebKitApp that's built on Chromium, EFL, GTK+, Qt, etc... ports in my
spare time, and started rolling out all Chromium, EFL, GTK+, Qt, etc...
patches that break automated tests in SuperWebKitApp, just giving
hyperlinks to my buildbots. That's pretty upsetting, isn't it?

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


Re: [webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects

2012-12-11 Thread Maciej Stachowiak

On Dec 11, 2012, at 7:34 PM, Ryosuke Niwa  wrote:

> On Tue, Dec 11, 2012 at 7:20 PM, Elliott Sprehn  wrote:
> Do you have an example of when this has occurred? It's good to have examples 
> if we want to prevent this in the future.
> 
> Yes. I'd rather not publicly humiliate someone on webkit-dev so I'll send you 
> a bug URL in private.
> 
> In this particular incident, a WebKit patch was rolled out due to a Chromium 
> UI test failure. The person who rolled out the patch didn't communicate any 
> information on the original bug from which the patch was landed. On the bug 
> where the rollout was made, the person left links to Chromium WebKit roll 
> patches but without any information regarding tests that failed. The only 
> reason I could follow his comments is because I used to work on Chromium. The 
> patch was subsequently rolled out in less than 20 minutes from the time the 
> person first provided any information about the test failure at all.
> 
> To make it even worse, the roll out was speculative, and the patch was found 
> innocent of causing the failure. The person promised to re-land the patch by 
> Monday and never came back to the bug. (Now you know where my previous email 
> came from).

In addition to the wrongnesses you pointed out, the combination of a 
speculative rollout for a downstream test failure (where clearly you should 
have the ability to test if the patch is really to blame) seems even more extra 
wrong.

I agree with not embarrassing people needlessly on webkit-dev, but someone 
should probably have a direct conversation about all this with the individual 
responsible.

 - Maciej

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


Re: [webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects

2012-12-11 Thread Mark Rowe

On 2012-12-11, at 19:34, Ryosuke Niwa  wrote:

> On Tue, Dec 11, 2012 at 7:20 PM, Elliott Sprehn  wrote:
> Do you have an example of when this has occurred? It's good to have examples 
> if we want to prevent this in the future.
> 
> Yes. I'd rather not publicly humiliate someone on webkit-dev so I'll send you 
> a bug URL in private.
> 
> In this particular incident, a WebKit patch was rolled out due to a Chromium 
> UI test failure. The person who rolled out the patch didn't communicate any 
> information on the original bug from which the patch was landed. On the bug 
> where the rollout was made, the person left links to Chromium WebKit roll 
> patches but without any information regarding tests that failed. The only 
> reason I could follow his comments is because I used to work on Chromium. The 
> patch was subsequently rolled out in less than 20 minutes from the time the 
> person first provided any information about the test failure at all.
> 
> To make it even worse, the roll out was speculative, and the patch was found 
> innocent of causing the failure. The person promised to re-land the patch by 
> Monday and never came back to the bug. (Now you know where my previous email 
> came from).

In my opinion, that sort of behavior is out of line with the behavior that we 
as a community expect from WebKit committers.

- Mark

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


Re: [webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects

2012-12-11 Thread Ryosuke Niwa
On Tue, Dec 11, 2012 at 7:20 PM, Elliott Sprehn wrote:

> Do you have an example of when this has occurred? It's good to have
> examples if we want to prevent this in the future.
>

Yes. I'd rather not publicly humiliate someone on webkit-dev so I'll send
you a bug URL in private.

In this particular incident, a WebKit patch was rolled out due to a
Chromium UI test failure. The person who rolled out the patch didn't
communicate any information on the original bug from which the patch was
landed. On the bug where the rollout was made, the person left links to
Chromium WebKit roll patches but without any information regarding tests
that failed. The only reason I could follow his comments is because I used
to work on Chromium. The patch was subsequently rolled out in less than 20
minutes from the time the person first provided any information about the
test failure at all.

To make it even worse, the roll out was speculative, and the patch was
found innocent of causing the failure. The person promised to re-land the
patch by Monday and never came back to the bug. (Now you know where my
previous email came from).

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


Re: [webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects

2012-12-11 Thread Elliott Sprehn
Do you have an example of when this has occurred? It's good to have
examples if we want to prevent this in the future.


On Tue, Dec 11, 2012 at 1:44 PM, Ryosuke Niwa  wrote:

> Hi,
>
> I've encountered a couple of incidences where people roll out patches
> saying that test X is failing on some downstream project Y without giving
> any details as to how those tests are failing and why that's a real WebKit
> regression we should care about.
>
> First off, I don't think we should be rolling out patches based solely on
> a downstream test unless there is a clear evidence that the failure is a
> real regression in WebKit that affects more than just the said downstream
> project. You may talk to the author and he or she might be nice enough to
> agree to roll out the patch, but I don't think we should be rolling out
> patches right away regardless.
>
> Second, if there is a clear WebKit regression, then you should communicate
> the following information at minimum:
>
>1. The exact location of the test that failed - URL, etc...
>2. The nature of the failure - assertion failure, feature it's
>testing, etc...
>3. The output (before and) after the failure started happening.
>4. Instructions to run the tests locally
>
> Without this, the author is left with no clue whatsoever
> to diagnose and/or fix the problem.
>
> - R. Niwa
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Proposal: Make Ninja the default build-system for build-webkit --chromium

2012-12-11 Thread Eric Seidel
Nevermind.  After further discussion with Nico, this can't work yet.

Ninja is currently configured to use a non-webkitty "out" build
directory, which is undoubtably going to confus some scripts/bots.

We'll try this again at a later time.  Apologies for the noise.

http://trac.webkit.org/changeset/137375

On Tue, Dec 11, 2012 at 3:33 PM, Eric Seidel  wrote:
> This is now complete:
> http://trac.webkit.org/changeset/137371
>
> I'm watching the bots.  Please contact me if you have any trouble.
>
> Thank you all for your feedback.
>
> On Mon, Dec 10, 2012 at 10:11 AM, Dirk Pranke  wrote:
>> On Mon, Dec 10, 2012 at 1:30 AM, Jochen Eisinger  wrote:
>>> Will the buildbots use ninja or the "native" build tools?
>>>
>>> My only concern is that we're catching problems with e.g. MSVS only after we
>>> roll the WebKit deps in chromium and one of the MSVS bots starts failing.
>>>
>>
>> Eric is only suggesting changing update-webkit and build-webkit, which
>> means that only the webkit.org bots would be affected. As long as the
>> chromium.org canaries are still using chromium checkouts (and the
>> native build systems), we'll still have coverage.
>>
>> Of course, your point is still valid for other scenarios where we
>> don't have coverage of what we use on the official builds, but as Nico
>> pointed out in a separate thread, so far this hasn't been a frequent
>> problem.
>>
>> -- Dirk
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Proposal: Make Ninja the default build-system for build-webkit --chromium

2012-12-11 Thread Eric Seidel
This is now complete:
http://trac.webkit.org/changeset/137371

I'm watching the bots.  Please contact me if you have any trouble.

Thank you all for your feedback.

On Mon, Dec 10, 2012 at 10:11 AM, Dirk Pranke  wrote:
> On Mon, Dec 10, 2012 at 1:30 AM, Jochen Eisinger  wrote:
>> Will the buildbots use ninja or the "native" build tools?
>>
>> My only concern is that we're catching problems with e.g. MSVS only after we
>> roll the WebKit deps in chromium and one of the MSVS bots starts failing.
>>
>
> Eric is only suggesting changing update-webkit and build-webkit, which
> means that only the webkit.org bots would be affected. As long as the
> chromium.org canaries are still using chromium checkouts (and the
> native build systems), we'll still have coverage.
>
> Of course, your point is still valid for other scenarios where we
> don't have coverage of what we use on the official builds, but as Nico
> pointed out in a separate thread, so far this hasn't been a frequent
> problem.
>
> -- Dirk
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Ryosuke Niwa
On Tue, Dec 11, 2012 at 2:17 PM, Peter Kasting wrote:

> On Tue, Dec 11, 2012 at 2:14 PM, Oliver Hunt  wrote:
>
>> I don't understand why anyone is _speculatively_ rolling out patches.
>>
>> You should only be rolling it out if you _know_ the patch is bad.
>>
>
> Sometimes something bad happens to the tree, the sheriff doesn't know
> which patch is responsible, and the change authors are not present to ask
> for help.  In a case like this the sheriff has to either do speculative
> rollouts or leave the tree broken.
>

Right.

Ideally, of course, change authors are around when something like this
> happens.  But maybe the bustage doesn't happen until much later, due to
> some subtle/latent issue, or maybe the change author is in fact
> irresponsible.
>

Given that some bots take 4-5 hours to cycle these days, it's hard to keep
eyes on all bots all the time. So things like this would happen.

Having said that, a *speculative *roll out should one's last report.
Rolling out a patch causes a lot of svn churn, increases bot cycle time,
etc... and should be avoided if the failure can be fixed easily.
Furthermore, it's often not too hard to test a rollout locally to see if it
actually fixes the problem as Oliver suggested.

-

In general, I feel that some people are too religious about keeping bots
green and too eager to roll out patches without trying to fix the failures
or even understanding the failures and are actively harmful to the project.

The main goal of continus build & test systems should be to help the
development of WebKit, not to run them for the sake of keeping them green.

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


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Dirk Pranke
The build cop / gardener / sheriff / whatever may not have local or
easy access to a bot that reproduces the problem ... rolling it out
might be the only feasible way to test in that case.

On Tue, Dec 11, 2012 at 2:20 PM, Oliver Hunt  wrote:
>
> On Dec 11, 2012, at 2:17 PM, Peter Kasting  wrote:
>
> On Tue, Dec 11, 2012 at 2:14 PM, Oliver Hunt  wrote:
>>
>> I don't understand why anyone is _speculatively_ rolling out patches.
>>
>> You should only be rolling it out if you _know_ the patch is bad.
>
>
> Sometimes something bad happens to the tree, the sheriff doesn't know which
> patch is responsible, and the change authors are not present to ask for
> help.  In a case like this the sheriff has to either do speculative rollouts
> or leave the tree broken.
>
> Ideally, of course, change authors are around when something like this
> happens.  But maybe the bustage doesn't happen until much later, due to some
> subtle/latent issue, or maybe the change author is in fact irresponsible.
>
>
> Or the sheriff could actually see if rolling out a patch locally fixes the
> problem.  I'm not sure why they're considering "not testing" to be a valid
> behaviour for someone who is ostensibly meant to be keeping things going in
> the face of people who aren't testing.
>
>
>
> PK
>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Peter Kasting
On Tue, Dec 11, 2012 at 2:20 PM, Oliver Hunt  wrote:

> Or the sheriff could actually see if rolling out a patch locally fixes the
> problem.  I'm not sure why they're considering "not testing" to be a valid
> behaviour for someone who is ostensibly meant to be keeping things going in
> the face of people who aren't testing.
>

If the sheriff is capable of testing locally, that's an option.  It's often
impossible, however, for the sheriff to test locally, e.g. if the bustage
is in a port he can't build.  Even when possible, it may take a
prohibitively long time to sync, build, and test, during which time the
tree is broken for everyone.  Cycling the main waterfall itself may
inconvenience the rest of the developer community less.  As usual, it's a
judgment call.

Again, I've spent many days as WebKit sheriff, and I've only done
speculative rollouts a couple of times, so I don't see this as a constant,
major problem.

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


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Oliver Hunt

On Dec 11, 2012, at 2:17 PM, Peter Kasting  wrote:

> On Tue, Dec 11, 2012 at 2:14 PM, Oliver Hunt  wrote:
> I don't understand why anyone is _speculatively_ rolling out patches.
> 
> You should only be rolling it out if you _know_ the patch is bad.
> 
> Sometimes something bad happens to the tree, the sheriff doesn't know which 
> patch is responsible, and the change authors are not present to ask for help. 
>  In a case like this the sheriff has to either do speculative rollouts or 
> leave the tree broken.
> 
> Ideally, of course, change authors are around when something like this 
> happens.  But maybe the bustage doesn't happen until much later, due to some 
> subtle/latent issue, or maybe the change author is in fact irresponsible.

Or the sheriff could actually see if rolling out a patch locally fixes the 
problem.  I'm not sure why they're considering "not testing" to be a valid 
behaviour for someone who is ostensibly meant to be keeping things going in the 
face of people who aren't testing.


> 
> PK 

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


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Peter Kasting
On Tue, Dec 11, 2012 at 2:14 PM, Oliver Hunt  wrote:

> I don't understand why anyone is _speculatively_ rolling out patches.
>
> You should only be rolling it out if you _know_ the patch is bad.
>

Sometimes something bad happens to the tree, the sheriff doesn't know which
patch is responsible, and the change authors are not present to ask for
help.  In a case like this the sheriff has to either do speculative
rollouts or leave the tree broken.

Ideally, of course, change authors are around when something like this
happens.  But maybe the bustage doesn't happen until much later, due to
some subtle/latent issue, or maybe the change author is in fact
irresponsible.

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


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Oliver Hunt
I don't understand why anyone is _speculatively_ rolling out patches.

You should only be rolling it out if you _know_ the patch is bad.

That said if you do rollout a random unrelated patch it is obviously your job 
to roll it back in.  You can't say "i thought this broke something, but i was 
wrong.  Here you can have that bug again."  There is no case where the original 
author needs to be involved as we've already determined that they did nothing 
wrong - the original breakage (of whatever form) was not caused by the patch 
you selected randomly, and they were not the author responsible for landing 
anything (eg. the rollout).

--Oliver

On Dec 11, 2012, at 1:21 PM, Peter Kasting  wrote:

> On Tue, Dec 11, 2012 at 1:19 PM, Emil A Eklund  wrote:
> > I don't understand your logic.  A patch landed, the sheriff thinks maybe it
> > was bad and rolls it out, then it turns out it was a red herring.  Why is it
> > not now the sheriff's responsibility to re-land?  After all, the patch was
> > landed originally by people who understood it and hasn't been seen to cause
> > any problems.
> 
> There might very well have been other changes that conflicts with it.
> If it applies cleanly then I agree with you that whoever rolled it out
> should reland it. If there are conflicts or if it requires merging in
> any way though I'd argue that the original author needs to get
> involved.
> 
> There are certainly cases where the original author needs to be involved, but 
> I'd be happy just saying this is a judgment call.  Usually rollouts happen 
> not long after a patch lands, and roll-ins happen not long after that.  In 
> those cases, most merge failures are trivial and mechanical and can easily be 
> handled by a conscientious sheriff who reads the relevant changes involved in 
> the conflicts.  Sometimes, of course, that's not true.  But sheriffs should 
> be biased towards "try to leave working patches in the tree".
> 
> PK
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev

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


[webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects

2012-12-11 Thread Ryosuke Niwa
Hi,

I've encountered a couple of incidences where people roll out patches
saying that test X is failing on some downstream project Y without giving
any details as to how those tests are failing and why that's a real WebKit
regression we should care about.

First off, I don't think we should be rolling out patches based solely on a
downstream test unless there is a clear evidence that the failure is a real
regression in WebKit that affects more than just the said downstream
project. You may talk to the author and he or she might be nice enough to
agree to roll out the patch, but I don't think we should be rolling out
patches right away regardless.

Second, if there is a clear WebKit regression, then you should communicate
the following information at minimum:

   1. The exact location of the test that failed - URL, etc...
   2. The nature of the failure - assertion failure, feature it's testing,
   etc...
   3. The output (before and) after the failure started happening.
   4. Instructions to run the tests locally

Without this, the author is left with no clue whatsoever to diagnose and/or
fix the problem.

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


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Ryosuke Niwa
On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund  wrote:

> If the build is broken and the gardener/build cop has a strong reason
> to suspect that it was caused by a specific patch and the author is
> unavailable then rolling that patch out is the right thing to do.


Sure. If the author isn't available via IRC & emails within a reasonable
time, and the regression is as serious as a build failure, then rolling out
the patch is quite reasonable even if it's speculative.

On the other hand, if the patch being rolled out turned out be not the
cause of whatever failure the person rolled it out for, then it should be
his/her responsibility to re-land the patch.

It might inconvenience the author but it is the responsibility of the
> author and reviewer to make sure the patch didn't break anything.
>

Sure.

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


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Peter Kasting
On Tue, Dec 11, 2012 at 1:19 PM, Emil A Eklund  wrote:

> > I don't understand your logic.  A patch landed, the sheriff thinks maybe
> it
> > was bad and rolls it out, then it turns out it was a red herring.  Why
> is it
> > not now the sheriff's responsibility to re-land?  After all, the patch
> was
> > landed originally by people who understood it and hasn't been seen to
> cause
> > any problems.
>
> There might very well have been other changes that conflicts with it.
> If it applies cleanly then I agree with you that whoever rolled it out
> should reland it. If there are conflicts or if it requires merging in
> any way though I'd argue that the original author needs to get
> involved.
>

There are certainly cases where the original author needs to be involved,
but I'd be happy just saying this is a judgment call.  Usually rollouts
happen not long after a patch lands, and roll-ins happen not long after
that.  In those cases, most merge failures are trivial and mechanical and
can easily be handled by a conscientious sheriff who reads the relevant
changes involved in the conflicts.  Sometimes, of course, that's not true.
 But sheriffs should be biased towards "try to leave working patches in the
tree".

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


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Emil A Eklund
On Tue, Dec 11, 2012 at 1:14 PM, Peter Kasting  wrote:
> On Tue, Dec 11, 2012 at 1:11 PM, Emil A Eklund  wrote:
>>
>> > That said, if your strong reason turned out to be incorrect, you should
>> > recommit the patch, no?
>>
>> That seems like a bad idea, someone that understands the patch should
>> recommit it. Ideally the original author.
>
>
> I don't understand your logic.  A patch landed, the sheriff thinks maybe it
> was bad and rolls it out, then it turns out it was a red herring.  Why is it
> not now the sheriff's responsibility to re-land?  After all, the patch was
> landed originally by people who understood it and hasn't been seen to cause
> any problems.

There might very well have been other changes that conflicts with it.
If it applies cleanly then I agree with you that whoever rolled it out
should reland it. If there are conflicts or if it requires merging in
any way though I'd argue that the original author needs to get
involved.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Peter Kasting
On Tue, Dec 11, 2012 at 1:11 PM, Emil A Eklund  wrote:

> > That said, if your strong reason turned out to be incorrect, you should
> recommit the patch, no?
>
> That seems like a bad idea, someone that understands the patch should
> recommit it. Ideally the original author.


I don't understand your logic.  A patch landed, the sheriff thinks maybe it
was bad and rolls it out, then it turns out it was a red herring.  Why is
it not now the sheriff's responsibility to re-land?  After all, the patch
was landed originally by people who understood it and hasn't been seen to
cause any problems.

On the occasions when I've had to roll-out to diagnose an issue, I've
always re-landed patches that it turns out weren't broken.  Not doing this
seems not only extremely rude but actively dangerous to the health of the
tree, since other changes may now be landed or near-landing that depend on
this change.

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


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Ojan Vafai
On Tue, Dec 11, 2012 at 1:11 PM, Emil A Eklund  wrote:

> On Tue, Dec 11, 2012 at 12:19 PM, Ojan Vafai  wrote:
> > On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund 
> wrote:
> >>
> >> I'll have to disagree with you here.
> >>
> >> If the build is broken and the gardener/build cop has a strong reason
> >> to suspect that it was caused by a specific patch and the author is
> >> unavailable then rolling that patch out is the right thing to do. It
> >
> >
> > "author is unavailable" is the key statement here.
>
> Indeed.
>
> > That said, if your strong reason turned out to be incorrect, you should
> recommit the patch, no?
>
> That seems like a bad idea, someone that understands the patch should
> recommit it. Ideally the original author.
>

If it needs manual patching then you need to include the original author,
but otherwise, I don't see why.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Emil A Eklund
On Tue, Dec 11, 2012 at 12:19 PM, Ojan Vafai  wrote:
> On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund  wrote:
>>
>> I'll have to disagree with you here.
>>
>> If the build is broken and the gardener/build cop has a strong reason
>> to suspect that it was caused by a specific patch and the author is
>> unavailable then rolling that patch out is the right thing to do. It
>
>
> "author is unavailable" is the key statement here.

Indeed.

> That said, if your strong reason turned out to be incorrect, you should 
> recommit the patch, no?

That seems like a bad idea, someone that understands the patch should
recommit it. Ideally the original author.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Ojan Vafai
On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund  wrote:

> I'll have to disagree with you here.
>
> If the build is broken and the gardener/build cop has a strong reason
> to suspect that it was caused by a specific patch and the author is
> unavailable then rolling that patch out is the right thing to do. It
>

"author is unavailable" is the key statement here. That said, if your
strong reason turned out to be incorrect, you should recommit the patch, no?


> might inconvenience the author but it is the responsibility of the
> author and reviewer to make sure the patch didn't break anything.
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Emil A Eklund
I'll have to disagree with you here.

If the build is broken and the gardener/build cop has a strong reason
to suspect that it was caused by a specific patch and the author is
unavailable then rolling that patch out is the right thing to do. It
might inconvenience the author but it is the responsibility of the
author and reviewer to make sure the patch didn't break anything.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

2012-12-11 Thread Ryosuke Niwa
Hi,

Please don't roll out patches speculatively unless that's the only way to
diagnose the problem. Even then you should really go talk to authors and
make sure they're okay with it. And please re-land patches that didn't
cause test failures or regressions promptly once you've fixed
or diagnosed the issue.

It's extremely rude to roll out someone else's patch speculatively and then
leave.

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


Re: [webkit-dev] Github vs. git.webkit.org

2012-12-11 Thread Jesus Sanchez-Palencia
Hi,

2012/12/4 Tor Arne Vestbø :
>
> Bill, what do you think about pushing the official SVN import to GitHub as
> well?
>
> tor arne

Any updates about this?

Cheers,
jesus


>
>>
>> So we might be able to rename the existing one and ask github to pull
>> our git.webkit.org  repository into
>>
>> github/WebKit/webkit.
>> Apparently Apache takes that way: https://github.com/apache
>> The "mirroring" icon indicates kind of official-ness.
>>
>> I don't know how long their mirroring delay is, though.
>>
>>
>>
>> On Sat, Dec 1, 2012 at 12:07 AM, Tor Arne Vestbø
>> mailto:tor.arne.ves...@digia.com>> wrote:
>>
>> On 11/28/12 16:55 , Adam Barth wrote:
>>
>> My sense is that the WebKit community would prefer that the
>> hashes in
>> GitHub match the hashes in git.webkit.org
>>  so that folks can more
>>
>> easily move branches between the two.  For my part, I've
>> switched over
>> to using GitHub exclusive of git.webkit.org
>> , so the the difference in
>>
>> hashes aren't an issue for me, but I can understand why they'd be
>> problematic for other people.
>>
>>
>> Yepp, agreed. Let's switch it over.
>>
>>
>> After the force-push, would you still be able to push updates
>> automatically?  If so, you can switch the hashes whenever is
>> convenient for you.  (It might be nice to announce the date/time
>> on
>> this list so that folks aren't taken by surprise.)
>>
>>
>> The mirror is also pushed to http://gitorious.org/webkit/__webkit
>> , which I was planning to keep
>>
>> as is for now, so that would mean setting up an extra mirroring for
>> the non-author-rewritten history :/ Also, the server I run this on
>> has a somewhat uncertain future. With that in mind it's probably
>> easier to just push directly from the same import that's pushed to
>> git.webkit.org , and make the GitHub mirror
>>
>> an official mirror?
>>
>> tor arne
>>
>> _
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org 
>> http://lists.webkit.org/__mailman/listinfo/webkit-dev
>> 
>>
>>
>>
>>
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] FYI: Console Message API tweaks.

2012-12-11 Thread Mike West
Hello, lovely WebKit folks.

If you never generate console messages for developers (seriously?), you can
stop reading.

Two changes have landed in the last week or so that I'd like to make you
aware of:

1. Call stacks or url/line numbers will now be autogenerated in the common
case (http://trac.webkit.org/changeset/136657). You should be able to get
away with simply calling `document->addConsoleMessage([MessageSource],
[MessageLevel], [Message])` or `console->addMessage([MessageSource],
[MessageLevel], [Message])` most of the time: if the document is parsing,
you'll get a line number. If it's being called from JavaScript, you'll get
a call stack. It won't solve all of the contextual issues, so you still
_can_ pass in lines or call stacks (see ContentSecurityPolicy for examples
of that), but I hope you won't have to think about it as often.

2. The sharpest among you will have noticed that the MessageType parameter
was missing from those examples above. Assuming
http://trac.webkit.org/changeset/137318 sticks, you don't need it anymore.
I've updated all the existing call sites, and I'm hopeful that the various
ports can check their ChromeClient usage to drop the parameter as well.
Once I have confirmation that it won't break ports, I'll drop the parameter
completely from the external interface.

That's it! The calls are slightly simpler, and should be slightly more
magical.

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