[webkit-dev] FYI: Console Message API tweaks.
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
Re: [webkit-dev] Github vs. git.webkit.org
Hi, 2012/12/4 Tor Arne Vestbø tor.arne.ves...@digia.com: 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 http://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ø tor.arne.ves...@digia.com 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 http://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 http://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 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 http://git.webkit.org, and make the GitHub mirror an official mirror? tor arne _ webkit-dev mailing list webkit-dev@lists.webkit.org mailto:webkit-dev@lists.webkit.org http://lists.webkit.org/__mailman/listinfo/webkit-dev 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] Please avoid rolling out patches speculatively and reland them ASAP if you had to
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] Please avoid rolling out patches speculatively and reland them ASAP if you had to
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
Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to
On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund e...@chromium.org 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
On Tue, Dec 11, 2012 at 12:19 PM, Ojan Vafai o...@chromium.org wrote: On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund e...@chromium.org 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
On Tue, Dec 11, 2012 at 1:11 PM, Emil A Eklund e...@chromium.org wrote: On Tue, Dec 11, 2012 at 12:19 PM, Ojan Vafai o...@chromium.org wrote: On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund e...@chromium.org 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
On Tue, Dec 11, 2012 at 1:11 PM, Emil A Eklund e...@chromium.org 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
On Tue, Dec 11, 2012 at 1:14 PM, Peter Kasting pkast...@chromium.org wrote: On Tue, Dec 11, 2012 at 1:11 PM, Emil A Eklund e...@chromium.org 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
On Tue, Dec 11, 2012 at 1:19 PM, Emil A Eklund e...@chromium.org 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
On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund e...@chromium.org 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
[webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects
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
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 pkast...@chromium.org wrote: On Tue, Dec 11, 2012 at 1:19 PM, Emil A Eklund e...@chromium.org 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
Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to
On Tue, Dec 11, 2012 at 2:14 PM, Oliver Hunt oli...@apple.com 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
On Dec 11, 2012, at 2:17 PM, Peter Kasting pkast...@chromium.org wrote: On Tue, Dec 11, 2012 at 2:14 PM, Oliver Hunt oli...@apple.com 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
On Tue, Dec 11, 2012 at 2:20 PM, Oliver Hunt oli...@apple.com 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
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 oli...@apple.com wrote: On Dec 11, 2012, at 2:17 PM, Peter Kasting pkast...@chromium.org wrote: On Tue, Dec 11, 2012 at 2:14 PM, Oliver Hunt oli...@apple.com 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
On Tue, Dec 11, 2012 at 2:17 PM, Peter Kasting pkast...@chromium.orgwrote: On Tue, Dec 11, 2012 at 2:14 PM, Oliver Hunt oli...@apple.com 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] Proposal: Make Ninja the default build-system for build-webkit --chromium
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 dpra...@chromium.org wrote: On Mon, Dec 10, 2012 at 1:30 AM, Jochen Eisinger joc...@chromium.org 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
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 e...@webkit.org 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 dpra...@chromium.org wrote: On Mon, Dec 10, 2012 at 1:30 AM, Jochen Eisinger joc...@chromium.org 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] Rolling out a patch requires a justification beyond a test failure in downstream projects
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 rn...@webkit.org 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] Rolling out a patch requires a justification beyond a test failure in downstream projects
On Tue, Dec 11, 2012 at 7:20 PM, Elliott Sprehn espr...@chromium.orgwrote: 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
On 2012-12-11, at 19:34, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Dec 11, 2012 at 7:20 PM, Elliott Sprehn espr...@chromium.org 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
On Dec 11, 2012, at 7:34 PM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Dec 11, 2012 at 7:20 PM, Elliott Sprehn espr...@chromium.org 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
On Tue, Dec 11, 2012 at 1:44 PM, Ryosuke Niwa rn...@webkit.org 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
Was this an isolated incident then? On Tue, Dec 11, 2012 at 8:39 PM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Dec 11, 2012 at 1:44 PM, Ryosuke Niwa rn...@webkit.org 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
On Tue, Dec 11, 2012 at 8:44 PM, David Levin le...@google.com 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
[webkit-dev] Behaviour of CapsLock in WebKit/Mac
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] Behaviour of CapsLock in WebKit/Mac
On 2012-12-11, at 21:24, Wez 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 (https://bugs.webkit.org/show_bug.cgi?id=18792). - Mark ___ 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
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 mr...@apple.com wrote: On 2012-12-11, at 21:24, Wez 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 (https://bugs.webkit.org/show_bug.cgi?id=18792). - 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
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 mr...@apple.comjavascript:_e({}, 'cvml', 'mr...@apple.com'); wrote: On 2012-12-11, at 21:24, Wez w...@chromium.org javascript:_e({}, '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 (https://bugs.webkit.org/show_bug.cgi?id=18792). - Mark ___ webkit-dev mailing list webkit-dev@lists.webkit.org javascript:_e({}, 'cvml', '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