Re: [webkit-dev] style queue
Yeah, the problem was it was spamming bugs with git errors. I'm going to make it more robust so it (hopefully!) won't spam before turning it back on. Adam On Wed, Feb 22, 2012 at 4:34 AM, Osztrogonac Csaba wrote: > Hi All, > > Style queue dead long time ago: > > Style Queue > 5 days, 16 hours ago > Status: Stopping Queue, reason: User terminated queue. > 799 pending > > Eric or Adam or anyone has access, could you kick it? > Thanks in advance. > > br, > Ossy > ___ > 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] style queue
Hi All, Style queue dead long time ago: Style Queue 5 days, 16 hours ago Status: Stopping Queue, reason: User terminated queue. 799 pending Eric or Adam or anyone has access, could you kick it? Thanks in advance. br, Ossy ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue
On Thu, Dec 9, 2010 at 12:42 AM, Eric Seidel wrote: > p.s. If anyone who actually likes sys-admining boxes would like to > admin some/all of these bots, Adam and I would be happy to show you > how. :) As you can tell, we suck at being sys-admins... By the way, thank you for keeping these bots going! As a novice contributor these bots save me so much time! I know sysadminning can be frustrating and thankless, so consider this an explicit "thank you". ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue
We believe the bots are all back functioning normally. (Including the sheriff-bot, thanks to Bill Siegrist's buildbot.py fixing this afternoon!) Thank you for your patience. -eric p.s. If anyone who actually likes sys-admining boxes would like to admin some/all of these bots, Adam and I would be happy to show you how. :) As you can tell, we suck at being sys-admins... On Tue, Dec 7, 2010 at 12:36 PM, Eric Seidel wrote: > I've killed the machine in question. It's out of disk space. The bots on > that machine (style-bot, sherriff-bot, cr-linux-ews) will be down until > tomorrow. > -eric > > On Tue, Dec 7, 2010 at 11:33 AM, Darin Adler wrote: >> >> On Dec 7, 2010, at 11:09 AM, Eric Seidel wrote: >> >> > Sorry about the recent noise from the style queue. Its git repo got >> > corrupted. Adam and I are working on getting to the machine and resetting >> > its git repo. >> >> Is there a way to stop the noise in the meantime? I’ve received many more >> email messages from bugs I’m cc’d on since you sent this 20 minutes ago. >> >> -- Darin >> > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue
I've killed the machine in question. It's out of disk space. The bots on that machine (style-bot, sherriff-bot, cr-linux-ews) will be down until tomorrow. -eric On Tue, Dec 7, 2010 at 11:33 AM, Darin Adler wrote: > On Dec 7, 2010, at 11:09 AM, Eric Seidel wrote: > > > Sorry about the recent noise from the style queue. Its git repo got > corrupted. Adam and I are working on getting to the machine and resetting > its git repo. > > Is there a way to stop the noise in the meantime? I’ve received many more > email messages from bugs I’m cc’d on since you sent this 20 minutes ago. > >-- Darin > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue
On Dec 7, 2010, at 11:09 AM, Eric Seidel wrote: > Sorry about the recent noise from the style queue. Its git repo got > corrupted. Adam and I are working on getting to the machine and resetting its > git repo. Is there a way to stop the noise in the meantime? I’ve received many more email messages from bugs I’m cc’d on since you sent this 20 minutes ago. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] style-queue
Sorry about the recent noise from the style queue. It's git repo got corrupted. Adam and I are working on getting to the machine and resetting it's git repo. -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
Wow. I have to admit I was skeptical. It's way too early to tell, but I *really* like the pass messages which make my job as a reviewer much easer: https://bugs.webkit.org/show_bug.cgi?id=31985#c3 Thank you very much for putting this together Adam! -eric On Mon, Nov 30, 2009 at 4:03 PM, Adam Barth wrote: > The queue is now running and caught up to the top of the review queue. > Please let me know if we should adjust anything. > > Thanks, > Adam > > > On Sat, Nov 28, 2009 at 2:21 AM, Adam Barth wrote: > > One of the bots that Eric and I have been working on is about to come > > online. This bot is a "style bot" that runs check-webkit-style on > > patches that have been nominated for review. I'd like to ask your > > patience while we work out the kinks. > > > > You can skip the rest of this email if you're not interested in the > > nitty gritty. > > > > == Summary == > > > > The primary goal of the style-queue is to reduce review latency by (1) > > giving immediate feedback to contributors and (2) making human > > reviewer more efficient by relieving them of mechanical tasks (like > > asking for tabs to be replaced with spaces). > > > > The style-queue scans the set of patches that are pending-review for > > new patches. When it finds a new patch, the bot runs > > check-webkit-style on the patch. If check-webkit-style finds style > > errors, the bot comments on the bug. > > > > == Social Contract == > > > > The style-queue is purely informative. You're free to ignore it's > > comments. Ideally, the bot should be silent unless it has something > > interesting to say. If we decide that style-queue is too chatty, we > > can change check-webkit-style to report fewer errors. > > > > One corollary of this social contract is that the style-queue doesn't > > inform you when your patch passes the style check (because most > > patches should pass, right?). We can re-visit this design choice > > later if it turns out we really want to know when the check succeeds. > > > > == Response to Previous Feedback == > > > > When I floated this idea on webkit-dev previously (in the "bot-filled > > future" thread), I got a bunch of useful feedback. Here's how we > > incorporated that feedback into the current design: > > > > 1) "Adding an extra flags is going to cause confusion." The > > style-queue does not add any flags to Bugzilla. Instead of storing > > it's state in Bugzilla flags (like commit-queue does), we built an > > AppEngine web service to hold the queue's persistent state. Instead > > of indicating style errors with a negative flag, the bot adds a > > comment to the bug. > > > > 2) "What machines are going to be doing these tests, and on which > > platforms?" The style-queue runs on a machine in my apartment that > > runs Mac OS X. The style-queue is platform independent, so this is > > less of an issue for style-queue than it is for a build-queue. > > > > 3) "Which patches would this test?" The style-queue tests any patch > > marked "review?". This design avoids the need for additional flags or > > indications that a patch should be run through the queue. > > > > 4) "Running tests on an arbitrary patches [opens a security hole]." > > Instead of running check-webkit-style from it's own working copy, the > > style-queue runs check-webkit-style from a separate working copy. > > While it's true you could haxor my machine by committing an evil copy > > of check-webkit-style, I'm already running that risk whenever I type > > "svn up". > > > > == Frequently Asked Questions == > > > > Q1: If the style-queue doesn't complain, does that mean my patch has > > correct style? > > > > A1: Unfortunately, no. First of all, check-webkit-style has false > > negatives. Hopefully, the script will improve over time, but it will > > never be perfect. Second, the style-queue is only able to check > > patches that successfully download and apply to top-of-tree. If your > > patch does not apply to top-of-tree (e.g., because it depends on > > another patch), then style-queue won't say anything. > > > > Q2: How can I see whether style-queue processed my patch? > > > > A2: There isn't a great way to do this yet. The data is stored in the > > webkit-commit-queue.appspot.com web service, but we haven't built a > > front-end for it yet. If you have a strong stomach, you can look at > > the endpoint the bot itself uses > > > > http://webkit-commit-queue.appspot.com/patch-status/style-queue/12345 > > > > where 12345 is the ID of your attachment. If you'd like to have a > > better status display, feel free to hack on > > WebKitTools/QueueStatusServer and send me or Eric patches to review. > > In the long term, there's no reason we can't have an awesome UI. > > > > Q3: This bot is going to run amock! How can I keep track of all it's > > evil deeds? > > > > A3: If you want to see what the bots are doing, you can subscribe to > > the following email list: > > > > http://groups.google.com/group/webkit-bot-watchers > > >
Re: [webkit-dev] style-queue entering beta
The queue is now running and caught up to the top of the review queue. Please let me know if we should adjust anything. Thanks, Adam On Sat, Nov 28, 2009 at 2:21 AM, Adam Barth wrote: > One of the bots that Eric and I have been working on is about to come > online. This bot is a "style bot" that runs check-webkit-style on > patches that have been nominated for review. I'd like to ask your > patience while we work out the kinks. > > You can skip the rest of this email if you're not interested in the > nitty gritty. > > == Summary == > > The primary goal of the style-queue is to reduce review latency by (1) > giving immediate feedback to contributors and (2) making human > reviewer more efficient by relieving them of mechanical tasks (like > asking for tabs to be replaced with spaces). > > The style-queue scans the set of patches that are pending-review for > new patches. When it finds a new patch, the bot runs > check-webkit-style on the patch. If check-webkit-style finds style > errors, the bot comments on the bug. > > == Social Contract == > > The style-queue is purely informative. You're free to ignore it's > comments. Ideally, the bot should be silent unless it has something > interesting to say. If we decide that style-queue is too chatty, we > can change check-webkit-style to report fewer errors. > > One corollary of this social contract is that the style-queue doesn't > inform you when your patch passes the style check (because most > patches should pass, right?). We can re-visit this design choice > later if it turns out we really want to know when the check succeeds. > > == Response to Previous Feedback == > > When I floated this idea on webkit-dev previously (in the "bot-filled > future" thread), I got a bunch of useful feedback. Here's how we > incorporated that feedback into the current design: > > 1) "Adding an extra flags is going to cause confusion." The > style-queue does not add any flags to Bugzilla. Instead of storing > it's state in Bugzilla flags (like commit-queue does), we built an > AppEngine web service to hold the queue's persistent state. Instead > of indicating style errors with a negative flag, the bot adds a > comment to the bug. > > 2) "What machines are going to be doing these tests, and on which > platforms?" The style-queue runs on a machine in my apartment that > runs Mac OS X. The style-queue is platform independent, so this is > less of an issue for style-queue than it is for a build-queue. > > 3) "Which patches would this test?" The style-queue tests any patch > marked "review?". This design avoids the need for additional flags or > indications that a patch should be run through the queue. > > 4) "Running tests on an arbitrary patches [opens a security hole]." > Instead of running check-webkit-style from it's own working copy, the > style-queue runs check-webkit-style from a separate working copy. > While it's true you could haxor my machine by committing an evil copy > of check-webkit-style, I'm already running that risk whenever I type > "svn up". > > == Frequently Asked Questions == > > Q1: If the style-queue doesn't complain, does that mean my patch has > correct style? > > A1: Unfortunately, no. First of all, check-webkit-style has false > negatives. Hopefully, the script will improve over time, but it will > never be perfect. Second, the style-queue is only able to check > patches that successfully download and apply to top-of-tree. If your > patch does not apply to top-of-tree (e.g., because it depends on > another patch), then style-queue won't say anything. > > Q2: How can I see whether style-queue processed my patch? > > A2: There isn't a great way to do this yet. The data is stored in the > webkit-commit-queue.appspot.com web service, but we haven't built a > front-end for it yet. If you have a strong stomach, you can look at > the endpoint the bot itself uses > > http://webkit-commit-queue.appspot.com/patch-status/style-queue/12345 > > where 12345 is the ID of your attachment. If you'd like to have a > better status display, feel free to hack on > WebKitTools/QueueStatusServer and send me or Eric patches to review. > In the long term, there's no reason we can't have an awesome UI. > > Q3: This bot is going to run amock! How can I keep track of all it's > evil deeds? > > A3: If you want to see what the bots are doing, you can subscribe to > the following email list: > > http://groups.google.com/group/webkit-bot-watchers > > Whenever the bots touch a bug, they're supposed to add this address to > the CC list. This functionality is very new, so we might not have > every case covered yet. If you see the bot misbehaving, let me or > Eric know. We'll correct the issue ASAP. > > Q4: Checking style is fine and dandy, but I was hoping for a build-queue! > > A4: My next project is to get a build-queue working for Qt. The > build-queue will share a lot of infrastructure with the style-queue, > but making the style-queue was easier becau
Re: [webkit-dev] style-queue entering beta
fwiw, I'm not aware of any bad error reports that people are hitting in practice although as Adam mentioned it is possible for various reasons. If we hit any incorrect warnings with any frequency, we should disable them or fix them. Of course, there are a lots of WebKit style guidelines that it doesn't catch. > One basic reason the script isn't perfect is that it's doesn't have a > full C++ / Objective-C++ parser. If we could go this route, would we prefer it? It may be a really good thing. It would depend on how complicated the tool/code got. As far as this tool goes, it is derived from an open source version of what Google uses in house and then originally adapted for WebKit by Shinichiro Hamaji. It primarily relies on regex. In the past I wrote a closed source tool that used a parser to do style fixes in lots of ways, and I appreciated the exactness of the results. One nice thing about the current code is that it is pretty accessible to new folks. I don't think that was as true of the tool I wrote on top of the parser which examined and manipulated syntax trees. Dave PS In fact, some checks by various folks in WebKit were made to catch less things in order to avoid false negatives because noisy incorrect tools are just annoying (and imo frequently are justifiably ignored). On Sun, Nov 29, 2009 at 5:55 PM, Adam Barth wrote: > Yeah, I think improving the script would be great. I'm not actually > an expert on how it works internally, but I think David Levin is. > It's easy for the bot to pass a flag to the script if that would be > helpful. In general, I think we should give it a try and iterate to > remove the biggest pain points. > > Thanks! > Adam > > > On Sun, Nov 29, 2009 at 5:00 PM, Chris Jerdonek > wrote: > > On Sun, Nov 29, 2009 at 9:35 AM, Adam Barth wrote: > > > >> On Sun, Nov 29, 2009 at 9:26 AM, Chris Jerdonek > >> wrote: > >>> On Sat, Nov 28, 2009, Adam Barth wrote: > Hopefully, the script will improve over time, but it will > never be perfect. > >>> > >>> Can you elaborate on this? For example, are you saying there is a > >>> basic reason that the script will always have bugs? Without knowing > >>> too much about the script, it seems like it wouldn't be too hard to at > >>> least make the false negatives go away. Or are you simply saying that > >>> the guidelines and script will never fully capture what we mean by > >>> "correct style"? > >> > >> Does this mean you're volunteering to remove all the false positives > >> and false negatives? :) > > > > I was hoping to work on the script eventually, which is partly why I > > asked for elaboration. > > > > All that I meant above is that one could potentially disable (for the > > bot) the style tests that report false violations, or else reduce > > their confidence score. That way, if the style bot flags a patch, it > > is guaranteed to be meaningful without looking at the details of the > > report. This can only be done, though, if the problems with the > > script are not so basic that they affect most or many of the tests. > > > > (The reverse is not as straightforward, though. It does not seem as > > easy to change the script -- in a useful way -- so that if it reports > > that a patch has met the guidelines, then the patch really meets the > > guidelines.) > > > >> One basic reason the script isn't perfect is that it's doesn't have a > >> full C++ / Objective-C++ parser. > > > > If we could go this route, would we prefer it? > > > > --Chris > > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
Yeah, I think improving the script would be great. I'm not actually an expert on how it works internally, but I think David Levin is. It's easy for the bot to pass a flag to the script if that would be helpful. In general, I think we should give it a try and iterate to remove the biggest pain points. Thanks! Adam On Sun, Nov 29, 2009 at 5:00 PM, Chris Jerdonek wrote: > On Sun, Nov 29, 2009 at 9:35 AM, Adam Barth wrote: > >> On Sun, Nov 29, 2009 at 9:26 AM, Chris Jerdonek >> wrote: >>> On Sat, Nov 28, 2009, Adam Barth wrote: Hopefully, the script will improve over time, but it will never be perfect. >>> >>> Can you elaborate on this? For example, are you saying there is a >>> basic reason that the script will always have bugs? Without knowing >>> too much about the script, it seems like it wouldn't be too hard to at >>> least make the false negatives go away. Or are you simply saying that >>> the guidelines and script will never fully capture what we mean by >>> "correct style"? >> >> Does this mean you're volunteering to remove all the false positives >> and false negatives? :) > > I was hoping to work on the script eventually, which is partly why I > asked for elaboration. > > All that I meant above is that one could potentially disable (for the > bot) the style tests that report false violations, or else reduce > their confidence score. That way, if the style bot flags a patch, it > is guaranteed to be meaningful without looking at the details of the > report. This can only be done, though, if the problems with the > script are not so basic that they affect most or many of the tests. > > (The reverse is not as straightforward, though. It does not seem as > easy to change the script -- in a useful way -- so that if it reports > that a patch has met the guidelines, then the patch really meets the > guidelines.) > >> One basic reason the script isn't perfect is that it's doesn't have a >> full C++ / Objective-C++ parser. > > If we could go this route, would we prefer it? > > --Chris > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
On Sun, Nov 29, 2009 at 9:35 AM, Adam Barth wrote: > On Sun, Nov 29, 2009 at 9:26 AM, Chris Jerdonek > wrote: >> On Sat, Nov 28, 2009, Adam Barth wrote: >>> Hopefully, the script will improve over time, but it will >>> never be perfect. >> >> Can you elaborate on this? For example, are you saying there is a >> basic reason that the script will always have bugs? Without knowing >> too much about the script, it seems like it wouldn't be too hard to at >> least make the false negatives go away. Or are you simply saying that >> the guidelines and script will never fully capture what we mean by >> "correct style"? > > Does this mean you're volunteering to remove all the false positives > and false negatives? :) I was hoping to work on the script eventually, which is partly why I asked for elaboration. All that I meant above is that one could potentially disable (for the bot) the style tests that report false violations, or else reduce their confidence score. That way, if the style bot flags a patch, it is guaranteed to be meaningful without looking at the details of the report. This can only be done, though, if the problems with the script are not so basic that they affect most or many of the tests. (The reverse is not as straightforward, though. It does not seem as easy to change the script -- in a useful way -- so that if it reports that a patch has met the guidelines, then the patch really meets the guidelines.) > One basic reason the script isn't perfect is that it's doesn't have a > full C++ / Objective-C++ parser. If we could go this route, would we prefer it? --Chris ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
On Sun, Nov 29, 2009 at 9:26 AM, Chris Jerdonek wrote: > On Sat, Nov 28, 2009, Adam Barth wrote: >> A1: Unfortunately, no. First of all, check-webkit-style has false >> negatives. > > It seems like this answers the different question, "If the style-queue > complains, does that mean my patch has incorrect style?" Yes, that too. I would call that a false positive, but you're right that check-webkit-style probably has both kinds of bugs. >> Hopefully, the script will improve over time, but it will >> never be perfect. > > Can you elaborate on this? For example, are you saying there is a > basic reason that the script will always have bugs? Without knowing > too much about the script, it seems like it wouldn't be too hard to at > least make the false negatives go away. Or are you simply saying that > the guidelines and script will never fully capture what we mean by > "correct style"? Does this mean you're volunteering to remove all the false positives and false negatives? :) One basic reason the script isn't perfect is that it's doesn't have a full C++ / Objective-C++ parser. I think it uses regular expressions. Anyway, I just wanted to set expectations that the tool might not be perfect. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
On Sat, Nov 28, 2009, Adam Barth wrote: > One of the bots that Eric and I have been working on is about to come > online. This bot is a "style bot" that runs check-webkit-style on > patches that have been nominated for review. This seems like a good effort, thanks. A couple minor comments below on the first question in your FAQ: > == Frequently Asked Questions == > > Q1: If the style-queue doesn't complain, does that mean my patch has > correct style? > > A1: Unfortunately, no. First of all, check-webkit-style has false > negatives. It seems like this answers the different question, "If the style-queue complains, does that mean my patch has incorrect style?" > Hopefully, the script will improve over time, but it will > never be perfect. Can you elaborate on this? For example, are you saying there is a basic reason that the script will always have bugs? Without knowing too much about the script, it seems like it wouldn't be too hard to at least make the false negatives go away. Or are you simply saying that the guidelines and script will never fully capture what we mean by "correct style"? > Second, the style-queue is only able to check > patches that successfully download and apply to top-of-tree. If your > patch does not apply to top-of-tree (e.g., because it depends on > another patch), then style-queue won't say anything. Thanks, --Chris ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
Maybe in the case of failure, provide a link to further information? Kenneth On Sun, Nov 29, 2009 at 3:44 AM, Maciej Stachowiak wrote: > > On Nov 28, 2009, at 10:53 AM, Adam Barth wrote: > > On Sat, Nov 28, 2009 at 10:21 AM, Maciej Stachowiak >> wrote: >> >>> On Nov 28, 2009, at 2:21 AM, Adam Barth wrote: >>> 1) "Adding an extra flags is going to cause confusion." The style-queue does not add any flags to Bugzilla. Instead of storing it's state in Bugzilla flags (like commit-queue does), we built an AppEngine web service to hold the queue's persistent state. Instead of indicating style errors with a negative flag, the bot adds a comment to the bug. >>> >>> It does seem that flags are noisy in an unappealing way, but it would be >>> much better (IMO) to store this information in the bugzilla database >>> instead >>> of externally. Is there any way we can do that? >>> >> >> From an implementation point of view, either way is easy now that >> we've got the infrastructure built. It's a question of which you'd >> prefer. >> >> Could we make a flag that is >>> hidden in the default UI, or use specially formatted comments that the >>> bot >>> knows how to recognize? >>> >> >> From my point of view, a hidden flag could work. We considered >> specially formatted comments, but that would make the bot more chatty. >> For example, if the style-queue has some internal error that prevents >> it from processing the patch, it wants to remember that, but it >> doesn't want to spam the bug with that information. >> >> I'm not sure representing all the state in Bugzilla is necessary. We >> should represent the "most interesting" state (e.g., pass / fail) >> there. For the rest, we can have a dashboard similar to >> build.webkit.org that shows the status of various patches before they >> are committed. I've started sketching something rough here: >> >> http://webkit-commit-queue.appspot.com/static/details.html >> > > Pass/fail status sounds fine to me. I agree that an error by the bot should > not be highly visible in the bug, as that is not as useful to the review and > submitter as Pass or Fail and Here's the Mistakes. > > > >> You can imagine clicking those squares to see the full log of what >> happened. For example, if the build failed on Qt, you might want to >> see the full output of build-webkit --qt, but we don't need to post >> all that to Bugzilla. The comment might just say: >> > > Looks like a decent design. > > > >> Patch 86912 did not build on Qt. Build log: >> >> http://webkit-commit-queue.appspot.com/patch-status/build-queue-qt/86912/all >> >> At a higher level, I'm sympathetic to Mark's concerns about what the >> system will look like when we have a number of bots. Bugzilla flags >> work well for receiving input from humans. There are lots of choices >> for how to present output. For example, another option is to have a >> bunch of colored squares next to each attachment that represent that >> patch's row on the dashboard. >> >> Adam >> > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > -- Kenneth Rohde Christiansen Technical Lead / Senior Software Engineer Qt Labs Americas, Nokia Technology Institute, INdT Phone +55 81 8895 6002 / E-mail kenneth.christiansen at openbossa.org ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
On Nov 28, 2009, at 10:53 AM, Adam Barth wrote: On Sat, Nov 28, 2009 at 10:21 AM, Maciej Stachowiak wrote: On Nov 28, 2009, at 2:21 AM, Adam Barth wrote: 1) "Adding an extra flags is going to cause confusion." The style-queue does not add any flags to Bugzilla. Instead of storing it's state in Bugzilla flags (like commit-queue does), we built an AppEngine web service to hold the queue's persistent state. Instead of indicating style errors with a negative flag, the bot adds a comment to the bug. It does seem that flags are noisy in an unappealing way, but it would be much better (IMO) to store this information in the bugzilla database instead of externally. Is there any way we can do that? From an implementation point of view, either way is easy now that we've got the infrastructure built. It's a question of which you'd prefer. Could we make a flag that is hidden in the default UI, or use specially formatted comments that the bot knows how to recognize? From my point of view, a hidden flag could work. We considered specially formatted comments, but that would make the bot more chatty. For example, if the style-queue has some internal error that prevents it from processing the patch, it wants to remember that, but it doesn't want to spam the bug with that information. I'm not sure representing all the state in Bugzilla is necessary. We should represent the "most interesting" state (e.g., pass / fail) there. For the rest, we can have a dashboard similar to build.webkit.org that shows the status of various patches before they are committed. I've started sketching something rough here: http://webkit-commit-queue.appspot.com/static/details.html Pass/fail status sounds fine to me. I agree that an error by the bot should not be highly visible in the bug, as that is not as useful to the review and submitter as Pass or Fail and Here's the Mistakes. You can imagine clicking those squares to see the full log of what happened. For example, if the build failed on Qt, you might want to see the full output of build-webkit --qt, but we don't need to post all that to Bugzilla. The comment might just say: Looks like a decent design. Patch 86912 did not build on Qt. Build log: http://webkit-commit-queue.appspot.com/patch-status/build-queue-qt/86912/all At a higher level, I'm sympathetic to Mark's concerns about what the system will look like when we have a number of bots. Bugzilla flags work well for receiving input from humans. There are lots of choices for how to present output. For example, another option is to have a bunch of colored squares next to each attachment that represent that patch's row on the dashboard. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
On Sat, Nov 28, 2009 at 10:21 AM, Maciej Stachowiak wrote: > On Nov 28, 2009, at 2:21 AM, Adam Barth wrote: >> 1) "Adding an extra flags is going to cause confusion." The >> style-queue does not add any flags to Bugzilla. Instead of storing >> it's state in Bugzilla flags (like commit-queue does), we built an >> AppEngine web service to hold the queue's persistent state. Instead >> of indicating style errors with a negative flag, the bot adds a >> comment to the bug. > > It does seem that flags are noisy in an unappealing way, but it would be > much better (IMO) to store this information in the bugzilla database instead > of externally. Is there any way we can do that? >From an implementation point of view, either way is easy now that we've got the infrastructure built. It's a question of which you'd prefer. > Could we make a flag that is > hidden in the default UI, or use specially formatted comments that the bot > knows how to recognize? >From my point of view, a hidden flag could work. We considered specially formatted comments, but that would make the bot more chatty. For example, if the style-queue has some internal error that prevents it from processing the patch, it wants to remember that, but it doesn't want to spam the bug with that information. I'm not sure representing all the state in Bugzilla is necessary. We should represent the "most interesting" state (e.g., pass / fail) there. For the rest, we can have a dashboard similar to build.webkit.org that shows the status of various patches before they are committed. I've started sketching something rough here: http://webkit-commit-queue.appspot.com/static/details.html You can imagine clicking those squares to see the full log of what happened. For example, if the build failed on Qt, you might want to see the full output of build-webkit --qt, but we don't need to post all that to Bugzilla. The comment might just say: Patch 86912 did not build on Qt. Build log: http://webkit-commit-queue.appspot.com/patch-status/build-queue-qt/86912/all At a higher level, I'm sympathetic to Mark's concerns about what the system will look like when we have a number of bots. Bugzilla flags work well for receiving input from humans. There are lots of choices for how to present output. For example, another option is to have a bunch of colored squares next to each attachment that represent that patch's row on the dashboard. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
Sounds like a good idea in general. Some comments below: On Nov 28, 2009, at 2:21 AM, Adam Barth wrote: One of the bots that Eric and I have been working on is about to come online. This bot is a "style bot" that runs check-webkit-style on patches that have been nominated for review. I'd like to ask your patience while we work out the kinks. You can skip the rest of this email if you're not interested in the nitty gritty. == Summary == The primary goal of the style-queue is to reduce review latency by (1) giving immediate feedback to contributors and (2) making human reviewer more efficient by relieving them of mechanical tasks (like asking for tabs to be replaced with spaces). The style-queue scans the set of patches that are pending-review for new patches. When it finds a new patch, the bot runs check-webkit-style on the patch. If check-webkit-style finds style errors, the bot comments on the bug. == Social Contract == The style-queue is purely informative. You're free to ignore it's comments. Ideally, the bot should be silent unless it has something interesting to say. If we decide that style-queue is too chatty, we can change check-webkit-style to report fewer errors. One corollary of this social contract is that the style-queue doesn't inform you when your patch passes the style check (because most patches should pass, right?). We can re-visit this design choice later if it turns out we really want to know when the check succeeds. To actually relieve reviewers of the burden of manual style review, it seems like the bot would have to record when a patch passes the style check. Otherwise, how can you tell whether a patch has passed the check, or if the bot just hasn't run on it yet? Would anyone object to seeing the information that a patch has passed an automated style check in a comment? == Response to Previous Feedback == When I floated this idea on webkit-dev previously (in the "bot-filled future" thread), I got a bunch of useful feedback. Here's how we incorporated that feedback into the current design: 1) "Adding an extra flags is going to cause confusion." The style-queue does not add any flags to Bugzilla. Instead of storing it's state in Bugzilla flags (like commit-queue does), we built an AppEngine web service to hold the queue's persistent state. Instead of indicating style errors with a negative flag, the bot adds a comment to the bug. It does seem that flags are noisy in an unappealing way, but it would be much better (IMO) to store this information in the bugzilla database instead of externally. Is there any way we can do that? Could we make a flag that is hidden in the default UI, or use specially formatted comments that the bot knows how to recognize? 2) "What machines are going to be doing these tests, and on which platforms?" The style-queue runs on a machine in my apartment that runs Mac OS X. The style-queue is platform independent, so this is less of an issue for style-queue than it is for a build-queue. 3) "Which patches would this test?" The style-queue tests any patch marked "review?". This design avoids the need for additional flags or indications that a patch should be run through the queue. 4) "Running tests on an arbitrary patches [opens a security hole]." Instead of running check-webkit-style from it's own working copy, the style-queue runs check-webkit-style from a separate working copy. While it's true you could haxor my machine by committing an evil copy of check-webkit-style, I'm already running that risk whenever I type "svn up". == Frequently Asked Questions == Q1: If the style-queue doesn't complain, does that mean my patch has correct style? A1: Unfortunately, no. First of all, check-webkit-style has false negatives. Hopefully, the script will improve over time, but it will never be perfect. Second, the style-queue is only able to check patches that successfully download and apply to top-of-tree. If your patch does not apply to top-of-tree (e.g., because it depends on another patch), then style-queue won't say anything. Q2: How can I see whether style-queue processed my patch? A2: There isn't a great way to do this yet. The data is stored in the webkit-commit-queue.appspot.com web service, but we haven't built a front-end for it yet. If you have a strong stomach, you can look at the endpoint the bot itself uses http://webkit-commit-queue.appspot.com/patch-status/style-queue/12345 where 12345 is the ID of your attachment. If you'd like to have a better status display, feel free to hack on WebKitTools/QueueStatusServer and send me or Eric patches to review. In the long term, there's no reason we can't have an awesome UI. Q3: This bot is going to run amock! How can I keep track of all it's evil deeds? A3: If you want to see what the bots are doing, you can subscribe to the following email list: http://groups.google.com/group/webkit-bot-watchers Whenever the bots touch a bug, they're suppose
[webkit-dev] style-queue entering beta
One of the bots that Eric and I have been working on is about to come online. This bot is a "style bot" that runs check-webkit-style on patches that have been nominated for review. I'd like to ask your patience while we work out the kinks. You can skip the rest of this email if you're not interested in the nitty gritty. == Summary == The primary goal of the style-queue is to reduce review latency by (1) giving immediate feedback to contributors and (2) making human reviewer more efficient by relieving them of mechanical tasks (like asking for tabs to be replaced with spaces). The style-queue scans the set of patches that are pending-review for new patches. When it finds a new patch, the bot runs check-webkit-style on the patch. If check-webkit-style finds style errors, the bot comments on the bug. == Social Contract == The style-queue is purely informative. You're free to ignore it's comments. Ideally, the bot should be silent unless it has something interesting to say. If we decide that style-queue is too chatty, we can change check-webkit-style to report fewer errors. One corollary of this social contract is that the style-queue doesn't inform you when your patch passes the style check (because most patches should pass, right?). We can re-visit this design choice later if it turns out we really want to know when the check succeeds. == Response to Previous Feedback == When I floated this idea on webkit-dev previously (in the "bot-filled future" thread), I got a bunch of useful feedback. Here's how we incorporated that feedback into the current design: 1) "Adding an extra flags is going to cause confusion." The style-queue does not add any flags to Bugzilla. Instead of storing it's state in Bugzilla flags (like commit-queue does), we built an AppEngine web service to hold the queue's persistent state. Instead of indicating style errors with a negative flag, the bot adds a comment to the bug. 2) "What machines are going to be doing these tests, and on which platforms?" The style-queue runs on a machine in my apartment that runs Mac OS X. The style-queue is platform independent, so this is less of an issue for style-queue than it is for a build-queue. 3) "Which patches would this test?" The style-queue tests any patch marked "review?". This design avoids the need for additional flags or indications that a patch should be run through the queue. 4) "Running tests on an arbitrary patches [opens a security hole]." Instead of running check-webkit-style from it's own working copy, the style-queue runs check-webkit-style from a separate working copy. While it's true you could haxor my machine by committing an evil copy of check-webkit-style, I'm already running that risk whenever I type "svn up". == Frequently Asked Questions == Q1: If the style-queue doesn't complain, does that mean my patch has correct style? A1: Unfortunately, no. First of all, check-webkit-style has false negatives. Hopefully, the script will improve over time, but it will never be perfect. Second, the style-queue is only able to check patches that successfully download and apply to top-of-tree. If your patch does not apply to top-of-tree (e.g., because it depends on another patch), then style-queue won't say anything. Q2: How can I see whether style-queue processed my patch? A2: There isn't a great way to do this yet. The data is stored in the webkit-commit-queue.appspot.com web service, but we haven't built a front-end for it yet. If you have a strong stomach, you can look at the endpoint the bot itself uses http://webkit-commit-queue.appspot.com/patch-status/style-queue/12345 where 12345 is the ID of your attachment. If you'd like to have a better status display, feel free to hack on WebKitTools/QueueStatusServer and send me or Eric patches to review. In the long term, there's no reason we can't have an awesome UI. Q3: This bot is going to run amock! How can I keep track of all it's evil deeds? A3: If you want to see what the bots are doing, you can subscribe to the following email list: http://groups.google.com/group/webkit-bot-watchers Whenever the bots touch a bug, they're supposed to add this address to the CC list. This functionality is very new, so we might not have every case covered yet. If you see the bot misbehaving, let me or Eric know. We'll correct the issue ASAP. Q4: Checking style is fine and dandy, but I was hoping for a build-queue! A4: My next project is to get a build-queue working for Qt. The build-queue will share a lot of infrastructure with the style-queue, but making the style-queue was easier because check-webkit-style is a lot faster than build-webkit. :) Q5: When are you flipping the switch? A5: Either today or tomorrow, depending on when I find a stretch of time to babysit the queue through whatever initial troubles it has. My working copy of the style-queue is about five patches ahead of trunk. It would be nice to have those patches committed before flippi
