Re: [webkit-dev] Frustrated at inconsiderate behavior
On Wed, Jul 7, 2010 at 7:29 PM, Maciej Stachowiak m...@apple.com wrote: One negative externality is that it sometimes makes people excessively upset about tree redness, and sometimes makes them want to fix redness in a way that papers over problems (e.g. by adding to the skipped list). If that's the main negative of the commit-queue, we can make it as agressive at landing patches as we want. We started out with a very conservative design, so there are a bunch of knobs to turn: 1) Currently the commit-queue only lands a patch if there were no commits between when it updated its working copy and when the tests finish. This check is to avoid the possibility of another change sneaking in that causes problems (e.g., a change to another file that conflicts semantically). The error case is probably very rare and might not be worth worrying about, but generally mean the commit-queue doesn't land during times when the tree is busy. 2) Currently the commit-queue only lands if it gets a 100% passing run of all the tests. We could instead change it to land if there are no new test failures as a result of applying the patch. This check is to avoid introducing new failures that are masked by existing failures. 3) Currently the commit-queue lands each patch individually. We could instead stack up a bunch of commits in a git branch and build/test them all at once. (Potentially this could be a massive increase in throughput.) We do this to space out the commits so that they generally get separate buildbot runs, which makes failures easier to localize. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Thu, Jul 8, 2010 at 7:54 PM, Adam Barth aba...@webkit.org wrote: 2) Currently the commit-queue only lands if it gets a 100% passing run of all the tests. We could instead change it to land if there are no new test failures as a result of applying the patch. This check is to avoid introducing new failures that are masked by existing failures. commit-queue has been pushing stuff all day and many core bots have been red since yesterday. How is this possible? Oh, and might this serve as a ping for whoever set the trees on fire... at least some of it seems related to the refPtr work that's been going on (see https://bugs.webkit.org/show_bug.cgi?id=41823) 3) Currently the commit-queue lands each patch individually. We could instead stack up a bunch of commits in a git branch and build/test them all at once. (Potentially this could be a massive increase in throughput.) We do this to space out the commits so that they generally get separate buildbot runs, which makes failures easier to localize. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Thu, Jul 8, 2010 at 10:58 AM, Xan Lopez x...@gnome.org wrote: On Thu, Jul 8, 2010 at 7:54 PM, Adam Barth aba...@webkit.org wrote: 2) Currently the commit-queue only lands if it gets a 100% passing run of all the tests. We could instead change it to land if there are no new test failures as a result of applying the patch. This check is to avoid introducing new failures that are masked by existing failures. commit-queue has been pushing stuff all day and many core bots have been red since yesterday. How is this possible? Originally, we had the commit-queue to land only if all the core builders were green. One recent change we made to make the commit-queue more agressive is to land when the commit-queue itself sees 100% passing tests on itself. When we were waiting for all the core builders to be green, the commit-queue usually had to wait for me or Eric or another contributor to clean up the entire tree and ping the folks who maintain bots that had gotten sick. My view is, generally, that the commit-queue should act like a contentious committer, which means it should act the way most committers act. Given that folks generally don't wait for all the bots to be green before committing, I felt this change was worth experimenting with. (Note that sheriff-bot still monitors all the core builders and alerts folks of bustage.) Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Thu, Jul 8, 2010 at 8:09 PM, Adam Barth aba...@webkit.org wrote: Originally, we had the commit-queue to land only if all the core builders were green. One recent change we made to make the commit-queue more agressive is to land when the commit-queue itself sees 100% passing tests on itself. When we were waiting for all the core builders to be green, the commit-queue usually had to wait for me or Eric or another contributor to clean up the entire tree and ping the folks who maintain bots that had gotten sick. My view is, generally, that the commit-queue should act like a contentious committer, which means it should act the way most committers act. Given that folks generally don't wait for all the bots to be green before committing, I felt this change was worth experimenting with. (Note that sheriff-bot still monitors all the core builders and alerts folks of bustage.) I see, I interpreted you meant it saw 100% passing tests in the core bots (as it used to be). The new behavior seems to make it easier to go back to how things were before, when as soon as one port (say GTK+) breaks, people will keep piling things on top and if nobody is around at that moment then it's much harder to figure out what happened, who was responsible, etc, because you only get clear data of the first breakage. So, not a fan. Of course I see the point that you make about most people committing manually doing the same thing, but maybe we should create a rule of not committing code touching all ports if any of the trees is red. That would improve things, I feel this switch won't do that. Xan Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Fri, Jul 9, 2010 at 2:14 AM, Xan Lopez x...@gnome.org wrote: On Thu, Jul 8, 2010 at 8:09 PM, Adam Barth aba...@webkit.org wrote: Originally, we had the commit-queue to land only if all the core builders were green. One recent change we made to make the commit-queue more agressive is to land when the commit-queue itself sees 100% passing tests on itself. When we were waiting for all the core builders to be green, the commit-queue usually had to wait for me or Eric or another contributor to clean up the entire tree and ping the folks who maintain bots that had gotten sick. My view is, generally, that the commit-queue should act like a contentious committer, which means it should act the way most committers act. Given that folks generally don't wait for all the bots to be green before committing, I felt this change was worth experimenting with. (Note that sheriff-bot still monitors all the core builders and alerts folks of bustage.) I see, I interpreted you meant it saw 100% passing tests in the core bots (as it used to be). The new behavior seems to make it easier to go back to how things were before, when as soon as one port (say GTK+) breaks, people will keep piling things on top and if nobody is around at that moment then it's much harder to figure out what happened, who was responsible, etc, because you only get clear data of the first breakage. So, not a fan. Of course I see the point that you make about most people committing manually doing the same thing, but maybe we should create a rule of not committing code touching all ports if any of the trees is red. That would improve things, I feel this switch won't do that. I think the point that's been made in this thread is that whatever policy the commit queue uses should be in line with whatever policies all committers should be using. Otherwise you run into the problems Maciej noted. If you think we should all follow such a policy, well that's an entirely different topic. And one that seems to keep coming up. If you want to raise it again, you should probably start by looking at the archives. J ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Thu, Jul 8, 2010 at 11:38 PM, Jeremy Orlow jor...@chromium.org wrote: I think the point that's been made in this thread is that whatever policy the commit queue uses should be in line with whatever policies all committers should be using. Otherwise you run into the problems Maciej noted. I don't see this particular point being made by anyone in the thread. Who are you referring to? In any case I'm not quite sure I follow the logic of downgrading machines to a lesser standard of quality just because this is what humans tend to do. If you think we should all follow such a policy, well that's an entirely different topic. And one that seems to keep coming up. If you want to raise it again, you should probably start by looking at the archives. Fair enough. My point was mostly about undoing the change to cq, not creating new rules for committers in general. J ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Thu, Jul 8, 2010 at 14:55, Xan Lopez x...@gnome.org wrote: On Thu, Jul 8, 2010 at 11:38 PM, Jeremy Orlow jor...@chromium.org wrote: I think the point that's been made in this thread is that whatever policy the commit queue uses should be in line with whatever policies all committers should be using. Otherwise you run into the problems Maciej noted. I don't see this particular point being made by anyone in the thread. Who are you referring to? In any case I'm not quite sure I follow the logic of downgrading machines to a lesser standard of quality just because this is what humans tend to do. If you think we should all follow such a policy, well that's an entirely different topic. And one that seems to keep coming up. If you want to raise it again, you should probably start by looking at the archives. Fair enough. My point was mostly about undoing the change to cq, not creating new rules for committers in general. As someone who doesn't have committer access, I like the change to the CQ. Before, I might have to wait 3 days for a patch to land since the CQ would wait for all the core builders to be green. Meanwhile, committers see that the CQ is behind and commit directly instead, so the tree stays on fire and the CQ gets even further behind. I don't care if the commit queue behavior is reverted, but if it is, it'd be really nice if committers tried to follow the same commit policy as the bot. J ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Fri, Jul 9, 2010 at 12:30 AM, Daniel Cheng dch...@chromium.org wrote: As someone who doesn't have committer access, I like the change to the CQ. Before, I might have to wait 3 days for a patch to land since the CQ would wait for all the core builders to be green. Meanwhile, committers see that the CQ is behind and commit directly instead, so the tree stays on fire and the CQ gets even further behind. I don't care if the commit queue behavior is reverted, but if it is, it'd be really nice if committers tried to follow the same commit policy as the bot. Aren't you essentially saying that you like the change because it allows you to join the fun of adding code to already broken trees? Again, I don't see how this helps us to move towards as-greener-as-possible trees. If core-builders are repeatedly red for days on end this is something we should address instead of adding workarounds so that people can keep working without paying attention to it. It has been noted that this is an often visited topic and off-topic to the thread, though. Xan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Thu, Jul 8, 2010 at 15:38, Xan Lopez x...@gnome.org wrote: Aren't you essentially saying that you like the change because it allows you to join the fun of adding code to already broken trees? Again, I don't see how this helps us to move towards as-greener-as-possible trees. If core-builders are repeatedly red for days on end this is something we should address instead of adding workarounds so that people can keep working without paying attention to it. It has been noted that this is an often visited topic and off-topic to the thread, though. Xan What I'm saying is that if the commit queue behavior is reverted, there ought to be some sort of policy (strong suggestion? rule?) in place for committers to discourage committing on top of a red tree as well. Maybe there's already one, but it's not strongly enforced and it's very easy to ignore if you just want to land your patch and start working on something else. For what it's worth, it was especially frustrating for me since I was only updating an interface in the Chromium platform layer and the 3 day delay blocked me from landing any of the associated patches on the Chromium side. Daniel ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
What I noticed wrt the commit queue is that when a core build is broken continuously for days (like what happened with the Chromium Linux bot earlier this week for example), it causes big problems for the commit queue. Perhaps a 12 hour rule would be sufficient -- the cq doesn't submit if a builder is red unless if the builder been red for = 12 hours. (Of course, someone would have to code and I'm not sure if this could be easily this could be determined). Also, we'd have to be aggressive about what btos would be included in this list. dave On Thu, Jul 8, 2010 at 3:49 PM, Daniel Cheng dch...@chromium.org wrote: On Thu, Jul 8, 2010 at 15:38, Xan Lopez x...@gnome.org wrote: Aren't you essentially saying that you like the change because it allows you to join the fun of adding code to already broken trees? Again, I don't see how this helps us to move towards as-greener-as-possible trees. If core-builders are repeatedly red for days on end this is something we should address instead of adding workarounds so that people can keep working without paying attention to it. It has been noted that this is an often visited topic and off-topic to the thread, though. Xan What I'm saying is that if the commit queue behavior is reverted, there ought to be some sort of policy (strong suggestion? rule?) in place for committers to discourage committing on top of a red tree as well. Maybe there's already one, but it's not strongly enforced and it's very easy to ignore if you just want to land your patch and start working on something else. For what it's worth, it was especially frustrating for me since I was only updating an interface in the Chromium platform layer and the 3 day delay blocked me from landing any of the associated patches on the Chromium side. Daniel ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
Thank you for fixing the problem. Did you try talking to Alexey directly about this? Or to someone else who may be familiar with the situation? It's usually better to try steps like that before calling someone out on the mailing list. And if you do need to bring something to wider attention, perhaps you could consider a less strident tone. In the WebKit project, we prefer to resolve disputes calmly and respectfully, especially among longtime valued contributors such as Alexey and yourself. I realize that at times, contributors can feel frustrated, but let's try to keep the project mailing list a happy place. Sincerely, Maciej On Jul 7, 2010, at 12:47 AM, Adam Barth wrote: If you're tired of my complaining about the tree being red, you can skip this message. Today Alexey checked in http://trac.webkit.org/changeset/62576, which broke two tests on every port. 12 hours later, these failures remained in the tree until I cleaned them up. This mess could have been avoided in a number of ways: 1) He could have run-webkit-tests before committing his change. 2) If he didn't have time to run the tests locally, he could have used the commit-queue to run-webkit-tests before it landed his patch. 3) He could have looked at the tree when sheriff-bot informed him that he might have broken Leopard Intel Debug by pinging him in #webkit and commenting on his bug: https://bugs.webkit.org/show_bug.cgi?id=41156#c8. Is this acceptable behavior? http://webkit.org/coding/contributing.html clearly says to run the layout tests using the run-webkit-tests script and make sure they all pass. That page also says: [[ In either case, your responsibility for the patch does not end with the patch landing in the tree. There may be regressions from your change or additional feedback from reviewers after the patch has landed. You can watch the tree at build.webkit.org to make sure your patch builds and passes tests on all platforms. It is your responsibility to be available should regressions arise and to respond to additional feedback that happens after a check-in. ]] Are there consequences for breaking these rules? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
07.07.2010, в 00:47, Adam Barth написал(а): If you're tired of my complaining about the tree being red, you can skip this message. I understand that you're frustrated, but I think that you're misinterpreting what happened. 1) He could have run-webkit-tests before committing his change. I did that. I made the mistake of running a wrong subset though (forgot that local xmlhttprequest tests were no longer in fast/dom). 2) If he didn't have time to run the tests locally, he could have used the commit-queue to run-webkit-tests before it landed his patch. Using commit-queue while it still doesn't provide correct svn blame is trading short term problems for longer term ones. Yes, one can still open a changeset by number, but having names really helps read svn blame. 3) He could have looked at the tree when sheriff-bot informed him that he might have broken Leopard Intel Debug by pinging him in #webkit and commenting on his bug: https://bugs.webkit.org/show_bug.cgi?id=41156#c8. I worked with Qt guys on fixing tests on Qt, and I watched the tree, which didn't show any sign of a problem from my patch at the time. Perhaps I'm starting to rely on sheriff bot too much - it didn't complain about any platforms besides Leopard Intel Debug. Leopard builder was seriously broken yesterday, so I didn't pay attention when it complained about a problem many hours later. In retrospect, that was a mistake. these failures remained in the tree until I cleaned them up I see that Tiger bot still has a unique failure, and will investigate it as soon as possible. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
Sorry for singling you out Alexey. I was just frustrated last night. On Wed, Jul 7, 2010 at 8:58 AM, Alexey Proskuryakov a...@webkit.org wrote: 07.07.2010, в 00:47, Adam Barth написал(а): If you're tired of my complaining about the tree being red, you can skip this message. I understand that you're frustrated, but I think that you're misinterpreting what happened. 1) He could have run-webkit-tests before committing his change. I did that. I made the mistake of running a wrong subset though (forgot that local xmlhttprequest tests were no longer in fast/dom). Maybe the root cause here is that the test suite takes too long to run? We can make run-webkit-tests faster... 2) If he didn't have time to run the tests locally, he could have used the commit-queue to run-webkit-tests before it landed his patch. Using commit-queue while it still doesn't provide correct svn blame is trading short term problems for longer term ones. Yes, one can still open a changeset by number, but having names really helps read svn blame. This problem is now fixed thanks to William Siegrist. Patches written by committers show up correctly in SVN blame and trac even if they're landed by the commit-queue. 3) He could have looked at the tree when sheriff-bot informed him that he might have broken Leopard Intel Debug by pinging him in #webkit and commenting on his bug: https://bugs.webkit.org/show_bug.cgi?id=41156#c8. I worked with Qt guys on fixing tests on Qt, and I watched the tree, which didn't show any sign of a problem from my patch at the time. Perhaps I'm starting to rely on sheriff bot too much - it didn't complain about any platforms besides Leopard Intel Debug. It only complains once to avoid spamming when it makes a mistake. Leopard builder was seriously broken yesterday, so I didn't pay attention when it complained about a problem many hours later. In retrospect, that was a mistake. I think we can improve this by having sheriff-bot say which tests broke. I bet if you saw these tests listed, you'd have realized what was going one. these failures remained in the tree until I cleaned them up I see that Tiger bot still has a unique failure, and will investigate it as soon as possible. Thanks. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
07.07.2010, в 9:18, Adam Barth написал(а): This problem is now fixed thanks to William Siegrist. Patches written by committers show up correctly in SVN blame and trac even if they're landed by the commit-queue. That's good to know, I'll try that next time. I think we can improve this by having sheriff-bot say which tests broke. I bet if you saw these tests listed, you'd have realized what was going one. That would be very useful indeed! It currently takes some effort to find out what exactly the bot complains about. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
I agree with Maciej's response, but at the same time I can understand why Adam was so frustrated. He (and others) have pointed out stuff like this on and off list over and over again with little apparent change... But that's not what I'm most worried about; why this was broken in the tree for 12 hours?? It seems like, at the very least, every single person who committed in that time frame should have taken a look at the tree after committing [1]. Why did not one of those people roll the patch out when it was clear that there were failures and Alexey wasn't in the process of fixing them? J [1] Yeah, in theory looking before hand and not landing while it's on fire is better, but I'm talking about the _bare minimum_ of what should have happened. On Thu, Jul 8, 2010 at 12:18 AM, Adam Barth aba...@webkit.org wrote: Sorry for singling you out Alexey. I was just frustrated last night. On Wed, Jul 7, 2010 at 8:58 AM, Alexey Proskuryakov a...@webkit.org wrote: 07.07.2010, в 00:47, Adam Barth написал(а): If you're tired of my complaining about the tree being red, you can skip this message. I understand that you're frustrated, but I think that you're misinterpreting what happened. 1) He could have run-webkit-tests before committing his change. I did that. I made the mistake of running a wrong subset though (forgot that local xmlhttprequest tests were no longer in fast/dom). Maybe the root cause here is that the test suite takes too long to run? We can make run-webkit-tests faster... 2) If he didn't have time to run the tests locally, he could have used the commit-queue to run-webkit-tests before it landed his patch. Using commit-queue while it still doesn't provide correct svn blame is trading short term problems for longer term ones. Yes, one can still open a changeset by number, but having names really helps read svn blame. This problem is now fixed thanks to William Siegrist. Patches written by committers show up correctly in SVN blame and trac even if they're landed by the commit-queue. 3) He could have looked at the tree when sheriff-bot informed him that he might have broken Leopard Intel Debug by pinging him in #webkit and commenting on his bug: https://bugs.webkit.org/show_bug.cgi?id=41156#c8. I worked with Qt guys on fixing tests on Qt, and I watched the tree, which didn't show any sign of a problem from my patch at the time. Perhaps I'm starting to rely on sheriff bot too much - it didn't complain about any platforms besides Leopard Intel Debug. It only complains once to avoid spamming when it makes a mistake. Leopard builder was seriously broken yesterday, so I didn't pay attention when it complained about a problem many hours later. In retrospect, that was a mistake. I think we can improve this by having sheriff-bot say which tests broke. I bet if you saw these tests listed, you'd have realized what was going one. these failures remained in the tree until I cleaned them up I see that Tiger bot still has a unique failure, and will investigate it as soon as possible. Thanks. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
07.07.2010, в 9:49, Jeremy Orlow написал(а): Why did not one of those people roll the patch out when it was clear that there were failures and Alexey wasn't in the process of fixing them? Just to avoid any misunderstanding, the best thing to do would have been to tell me about the problem on IRC. I wasn't around at midnight, when Adam noticed the problem, but I was on IRC for many hours after landing that patch. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Jul 7, 2010, at 9:45 AM, Alexey Proskuryakov wrote: I think we can improve this by having sheriff-bot say which tests broke. I bet if you saw these tests listed, you'd have realized what was going one. That would be very useful indeed! It currently takes some effort to find out what exactly the bot complains about. Another possibility is for it to link to the test results page when it reports a failure - not as obvious as reporting the failing tests inline, but this would make it very convenient to look into the details of the failure. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Thu, Jul 8, 2010 at 2:03 AM, Alexey Proskuryakov a...@webkit.org wrote: 07.07.2010, в 9:49, Jeremy Orlow написал(а): Why did not one of those people roll the patch out when it was clear that there were failures and Alexey wasn't in the process of fixing them? Just to avoid any misunderstanding, the best thing to do would have been to tell me about the problem on IRC. I wasn't around at midnight, when Adam noticed the problem, but I was on IRC for many hours after landing that patch. Surewell, why did neither happen? Can any committers who saw the redness but didn't work to resolve the problem comment on how the process broke down? J ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
Maybe I should complain this in a different threads, but recently the commit bot waiting time is way too long. Several times a patch of mine got the r+ and cq+ and it landed two days later. This is really frustrating. I am very tempted to use svn directly to commit patches, but that means the patch only gets tested in my local environments. Like one time my patch breaks the leopard bot, turns out the failed test is skipped on leopard, which is exactly my OS. If I land it through the commit bots, I could identify the issue earlier. If there is any way to improve the situation, I'd really appreciate it. mo On Wed, Jul 7, 2010 at 12:47 AM, Adam Barth aba...@webkit.org wrote: If you're tired of my complaining about the tree being red, you can skip this message. Today Alexey checked in http://trac.webkit.org/changeset/62576, which broke two tests on every port. 12 hours later, these failures remained in the tree until I cleaned them up. This mess could have been avoided in a number of ways: 1) He could have run-webkit-tests before committing his change. 2) If he didn't have time to run the tests locally, he could have used the commit-queue to run-webkit-tests before it landed his patch. 3) He could have looked at the tree when sheriff-bot informed him that he might have broken Leopard Intel Debug by pinging him in #webkit and commenting on his bug: https://bugs.webkit.org/show_bug.cgi?id=41156#c8. Is this acceptable behavior? http://webkit.org/coding/contributing.html clearly says to run the layout tests using the run-webkit-tests script and make sure they all pass. That page also says: [[ In either case, your responsibility for the patch does not end with the patch landing in the tree. There may be regressions from your change or additional feedback from reviewers after the patch has landed. You can watch the tree at build.webkit.org to make sure your patch builds and passes tests on all platforms. It is your responsibility to be available should regressions arise and to respond to additional feedback that happens after a check-in. ]] Are there consequences for breaking these rules? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Wed, Jul 7, 2010 at 6:50 PM, Mo, Zhenyao zhen...@gmail.com wrote: Maybe I should complain this in a different threads, but recently the commit bot waiting time is way too long. Several times a patch of mine got the r+ and cq+ and it landed two days later. This is really frustrating. I am very tempted to use svn directly to commit patches, but that means the patch only gets tested in my local environments. Like one time my patch breaks the leopard bot, turns out the failed test is skipped on leopard, which is exactly my OS. If I land it through the commit bots, I could identify the issue earlier. I agree they are closely related. A greener tree means a faster commit queue and a faster commit queue means less people subvert it and break the tree. The hard problem is figuring out how to fix the incentives so subverting the queue isn't so desirable. There is also a smaller but more concrete problem. Older bugs cut in front of newer bugs in the commit queue. So when the queue is moving slowly, patches with recent bug IDs could spend a long time getting bumped before finally landing. https://bugs.webkit.org/show_bug.cgi?id=41791 If there is any way to improve the situation, I'd really appreciate it. mo On Wed, Jul 7, 2010 at 12:47 AM, Adam Barth aba...@webkit.org wrote: If you're tired of my complaining about the tree being red, you can skip this message. Today Alexey checked in http://trac.webkit.org/changeset/62576, which broke two tests on every port. 12 hours later, these failures remained in the tree until I cleaned them up. This mess could have been avoided in a number of ways: 1) He could have run-webkit-tests before committing his change. 2) If he didn't have time to run the tests locally, he could have used the commit-queue to run-webkit-tests before it landed his patch. 3) He could have looked at the tree when sheriff-bot informed him that he might have broken Leopard Intel Debug by pinging him in #webkit and commenting on his bug: https://bugs.webkit.org/show_bug.cgi?id=41156#c8. Is this acceptable behavior? http://webkit.org/coding/contributing.html clearly says to run the layout tests using the run-webkit-tests script and make sure they all pass. That page also says: [[ In either case, your responsibility for the patch does not end with the patch landing in the tree. There may be regressions from your change or additional feedback from reviewers after the patch has landed. You can watch the tree at build.webkit.org to make sure your patch builds and passes tests on all platforms. It is your responsibility to be available should regressions arise and to respond to additional feedback that happens after a check-in. ]] Are there consequences for breaking these rules? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Jul 7, 2010, at 7:16 PM, Tony Gentilcore wrote: On Wed, Jul 7, 2010 at 6:50 PM, Mo, Zhenyao zhen...@gmail.com wrote: Maybe I should complain this in a different threads, but recently the commit bot waiting time is way too long. Several times a patch of mine got the r+ and cq+ and it landed two days later. This is really frustrating. I am very tempted to use svn directly to commit patches, but that means the patch only gets tested in my local environments. Like one time my patch breaks the leopard bot, turns out the failed test is skipped on leopard, which is exactly my OS. If I land it through the commit bots, I could identify the issue earlier. I agree they are closely related. A greener tree means a faster commit queue and a faster commit queue means less people subvert it and break the tree. The hard problem is figuring out how to fix the incentives so subverting the queue isn't so desirable. What do you mean by subvert the queue? The commit queue is a tool to streamline commits from contributors who do not have commit access to the repository. If you have the ability to commit you should not be using the commit queue to land your patches. --Oliver ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Wed, Jul 7, 2010 at 7:19 PM, Oliver Hunt oli...@apple.com wrote: On Jul 7, 2010, at 7:16 PM, Tony Gentilcore wrote: On Wed, Jul 7, 2010 at 6:50 PM, Mo, Zhenyao zhen...@gmail.com wrote: Maybe I should complain this in a different threads, but recently the commit bot waiting time is way too long. Several times a patch of mine got the r+ and cq+ and it landed two days later. This is really frustrating. I am very tempted to use svn directly to commit patches, but that means the patch only gets tested in my local environments. Like one time my patch breaks the leopard bot, turns out the failed test is skipped on leopard, which is exactly my OS. If I land it through the commit bots, I could identify the issue earlier. I agree they are closely related. A greener tree means a faster commit queue and a faster commit queue means less people subvert it and break the tree. The hard problem is figuring out how to fix the incentives so subverting the queue isn't so desirable. What do you mean by subvert the queue? The commit queue is a tool to streamline commits from contributors who do not have commit access to the repository. If you have the ability to commit you should not be using the commit queue to land your patches. That's not my understanding of the commit queue. I use the commit queue to land my patches when possible so that the patch receives further testing before it hits the tree and potentially affects a large number of contributors. Why do you think this is a bad idea? Is this preference codified somewhere (formally or informally)? - James --Oliver ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Wed, Jul 7, 2010 at 7:22 PM, James Robinson jam...@google.com wrote: On Wed, Jul 7, 2010 at 7:19 PM, Oliver Hunt oli...@apple.com wrote: On Jul 7, 2010, at 7:16 PM, Tony Gentilcore wrote: On Wed, Jul 7, 2010 at 6:50 PM, Mo, Zhenyao zhen...@gmail.com wrote: Maybe I should complain this in a different threads, but recently the commit bot waiting time is way too long. Several times a patch of mine got the r+ and cq+ and it landed two days later. This is really frustrating. I am very tempted to use svn directly to commit patches, but that means the patch only gets tested in my local environments. Like one time my patch breaks the leopard bot, turns out the failed test is skipped on leopard, which is exactly my OS. If I land it through the commit bots, I could identify the issue earlier. I agree they are closely related. A greener tree means a faster commit queue and a faster commit queue means less people subvert it and break the tree. The hard problem is figuring out how to fix the incentives so subverting the queue isn't so desirable. What do you mean by subvert the queue? The commit queue is a tool to streamline commits from contributors who do not have commit access to the repository. If you have the ability to commit you should not be using the commit queue to land your patches. That's not my understanding of the commit queue. I use the commit queue to land my patches when possible so that the patch receives further testing before it hits the tree and potentially affects a large number of contributors. Why do you think this is a bad idea? Is this preference codified somewhere (formally or informally)? Interesting. I'm a very new committer and it is possible I've completely misunderstood it. I thought of it as a useful tool to help make sure our patches don't break the tree. But given James' response I'm now very interested to hear what others think. - James --Oliver ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Jul 7, 2010, at 7:22 PM, James Robinson wrote: On Wed, Jul 7, 2010 at 7:19 PM, Oliver Hunt oli...@apple.com wrote: On Jul 7, 2010, at 7:16 PM, Tony Gentilcore wrote: On Wed, Jul 7, 2010 at 6:50 PM, Mo, Zhenyao zhen...@gmail.com wrote: Maybe I should complain this in a different threads, but recently the commit bot waiting time is way too long. Several times a patch of mine got the r+ and cq+ and it landed two days later. This is really frustrating. I am very tempted to use svn directly to commit patches, but that means the patch only gets tested in my local environments. Like one time my patch breaks the leopard bot, turns out the failed test is skipped on leopard, which is exactly my OS. If I land it through the commit bots, I could identify the issue earlier. I agree they are closely related. A greener tree means a faster commit queue and a faster commit queue means less people subvert it and break the tree. The hard problem is figuring out how to fix the incentives so subverting the queue isn't so desirable. What do you mean by subvert the queue? The commit queue is a tool to streamline commits from contributors who do not have commit access to the repository. If you have the ability to commit you should not be using the commit queue to land your patches. That's not my understanding of the commit queue. I use the commit queue to land my patches when possible so that the patch receives further testing before it hits the tree and potentially affects a large number of contributors. Why do you think this is a bad idea? Is this preference codified somewhere (formally or informally)? Previously the commit queue had a problem whereby it would show incorrect committer information (every patch committed by Eric). Now that is fixed - every patch submitted by a committer will show the proper committer, and those submitted by a non-committer will show up as committed by commit-queue. I think it's a choice of which approach to use. In addition to the obvious benefit of testing your patch for you, I think one positive externality of the commit queue is that people who use it are more motivated to keep the tree green. One negative externality is that it sometimes makes people excessively upset about tree redness, and sometimes makes them want to fix redness in a way that papers over problems (e.g. by adding to the skipped list). On the whole, I suspect the benefits of automated testing before landing probably outweigh these concerns. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Jul 7, 2010, at 7:26 PM, Jeremy Orlow wrote: On Thu, Jul 8, 2010 at 10:19 AM, Oliver Hunt oli...@apple.com wrote: On Jul 7, 2010, at 7:16 PM, Tony Gentilcore wrote: On Wed, Jul 7, 2010 at 6:50 PM, Mo, Zhenyao zhen...@gmail.com wrote: Maybe I should complain this in a different threads, but recently the commit bot waiting time is way too long. Several times a patch of mine got the r+ and cq+ and it landed two days later. This is really frustrating. I am very tempted to use svn directly to commit patches, but that means the patch only gets tested in my local environments. Like one time my patch breaks the leopard bot, turns out the failed test is skipped on leopard, which is exactly my OS. If I land it through the commit bots, I could identify the issue earlier. I agree they are closely related. A greener tree means a faster commit queue and a faster commit queue means less people subvert it and break the tree. The hard problem is figuring out how to fix the incentives so subverting the queue isn't so desirable. What do you mean by subvert the queue? The commit queue is a tool to streamline commits from contributors who do not have commit access to the repository. If you have the ability to commit you should not be using the commit queue to land your patches. That is the opinion of a few contributors, yes. But even when there were some big downsides to committers using the commit queue, there was definitely no definitive direction to not use the queue. And now that pretty much all the problems with the queue have been systematically eliminated, I would argue that--if anything--we should actually ask everyone to use the queue when practical. webkit-patch land-safely does the job of running the tests automatically, that said if you have commit privileges you should be running the tests yourself otherwise you're wasting the reviewers time. Pushing a patch through the normal review path will have it built on multiple platforms (though it is annoying that once you get r+ those builders don't run). The only benefit of the commit-queue (as i see it) is that it will make sure that the patch still applies in the many hours between it being reviewed and it being landed. My opinion is that if people want to use the commit-queue to land patches they should be happy to drop their commit privileges thus mooting this entire issue. --Oliver ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Jul 7, 2010, at 7:32 PM, Oliver Hunt wrote: webkit-patch land-safely does the job of running the tests automatically, that said if you have commit privileges you should be running the tests yourself otherwise you're wasting the reviewers time. Pushing a patch through the normal review path will have it built on multiple platforms (though it is annoying that once you get r+ those builders don't run). The only benefit of the commit-queue (as i see it) is that it will make sure that the patch still applies in the many hours between it being reviewed and it being landed. You can roll out the patch and work on something else, which might be a benefit to some. Also, the track record of the commit queue seems to be that people using it are less likely to break the tree in practice. I'm not naturally enthusiastic about the commit queue approach (I like to be around when my patch actually lands), but you can't argue with results. The only remaining downside (in my opinion) is sometimes making people overly frustrated when it's blocked, and occasionally leading to inelegant fixes for tree redness. My opinion is that if people want to use the commit-queue to land patches they should be happy to drop their commit privileges thus mooting this entire issue. That would probably make things worse on net, since the commit queue would then not mark the person as the committer in SVN. Personally, I don't think we need to either mandate or discourage use of the commit queue at this time. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Jul 7, 2010, at 7:42 PM, Maciej Stachowiak wrote: On Jul 7, 2010, at 7:32 PM, Oliver Hunt wrote: My opinion is that if people want to use the commit-queue to land patches they should be happy to drop their commit privileges thus mooting this entire issue. That would probably make things worse on net, since the commit queue would then not mark the person as the committer in SVN. On a side note, I'm pretty sure we could make the CQ rewrite even non-commiter authors. At some point we decided to only do committers, but I do not believe there is any technical reason preventing us from rewriting everyone. -Bill ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Frustrated at inconsiderate behavior
On Wed, Jul 7, 2010 at 9:46 PM, William Siegrist wsiegr...@apple.comwrote: On Jul 7, 2010, at 7:42 PM, Maciej Stachowiak wrote: On Jul 7, 2010, at 7:32 PM, Oliver Hunt wrote: My opinion is that if people want to use the commit-queue to land patches they should be happy to drop their commit privileges thus mooting this entire issue. That would probably make things worse on net, since the commit queue would then not mark the person as the committer in SVN. On a side note, I'm pretty sure we could make the CQ rewrite even non-commiter authors. At some point we decided to only do committers, but I do not believe there is any technical reason preventing us from rewriting everyone. I think that would be awesome. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
