Wow.  I have to admit I was skeptical.  It&#39;s way too early to tell, but I *really* like the pass messages which make my job as a reviewer much easer:<div><div><a href="https://bugs.webkit.org/show_bug.cgi?id=31985#c3">https://bugs.webkit.org/show_bug.cgi?id=31985#c3</a></div>
<div><br></div><div>Thank you very much for putting this together Adam!</div><div><br></div><div>-eric<br><br><div class="gmail_quote">On Mon, Nov 30, 2009 at 4:03 PM, Adam Barth <span dir="ltr">&lt;<a href="mailto:abarth@webkit.org">abarth@webkit.org</a>&gt;</span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">The queue is now running and caught up to the top of the review queue.<br>
 Please let me know if we should adjust anything.<br>
<br>
Thanks,<br>
<font color="#888888">Adam<br>
</font><div><div></div><div class="h5"><br>
<br>
On Sat, Nov 28, 2009 at 2:21 AM, Adam Barth &lt;<a href="mailto:abarth@webkit.org">abarth@webkit.org</a>&gt; wrote:<br>
&gt; One of the bots that Eric and I have been working on is about to come<br>
&gt; online.  This bot is a &quot;style bot&quot; that runs check-webkit-style on<br>
&gt; patches that have been nominated for review.  I&#39;d like to ask your<br>
&gt; patience while we work out the kinks.<br>
&gt;<br>
&gt; You can skip the rest of this email if you&#39;re not interested in the<br>
&gt; nitty gritty.<br>
&gt;<br>
&gt; == Summary ==<br>
&gt;<br>
&gt; The primary goal of the style-queue is to reduce review latency by (1)<br>
&gt; giving immediate feedback to contributors and (2) making human<br>
&gt; reviewer more efficient by relieving them of mechanical tasks (like<br>
&gt; asking for tabs to be replaced with spaces).<br>
&gt;<br>
&gt; The style-queue scans the set of patches that are pending-review for<br>
&gt; new patches.  When it finds a new patch, the bot runs<br>
&gt; check-webkit-style on the patch.  If check-webkit-style finds style<br>
&gt; errors, the bot comments on the bug.<br>
&gt;<br>
&gt; == Social Contract ==<br>
&gt;<br>
&gt; The style-queue is purely informative.  You&#39;re free to ignore it&#39;s<br>
&gt; comments.  Ideally, the bot should be silent unless it has something<br>
&gt; interesting to say.  If we decide that style-queue is too chatty, we<br>
&gt; can change check-webkit-style to report fewer errors.<br>
&gt;<br>
&gt; One corollary of this social contract is that the style-queue doesn&#39;t<br>
&gt; inform you when your patch passes the style check (because most<br>
&gt; patches should pass, right?).  We can re-visit this design choice<br>
&gt; later if it turns out we really want to know when the check succeeds.<br>
&gt;<br>
&gt; == Response to Previous Feedback ==<br>
&gt;<br>
&gt; When I floated this idea on webkit-dev previously (in the &quot;bot-filled<br>
&gt; future&quot; thread), I got a bunch of useful feedback.  Here&#39;s how we<br>
&gt; incorporated that feedback into the current design:<br>
&gt;<br>
&gt; 1) &quot;Adding an extra flags is going to cause confusion.&quot;  The<br>
&gt; style-queue does not add any flags to Bugzilla.  Instead of storing<br>
&gt; it&#39;s state in Bugzilla flags (like commit-queue does), we built an<br>
&gt; AppEngine web service to hold the queue&#39;s persistent state.  Instead<br>
&gt; of indicating style errors with a negative flag, the bot adds a<br>
&gt; comment to the bug.<br>
&gt;<br>
&gt; 2) &quot;What machines are going to be doing these tests, and on which<br>
&gt; platforms?&quot;  The style-queue runs on a machine in my apartment that<br>
&gt; runs Mac OS X.  The style-queue is platform independent, so this is<br>
&gt; less of an issue for style-queue than it is for a build-queue.<br>
&gt;<br>
&gt; 3) &quot;Which patches would this test?&quot;  The style-queue tests any patch<br>
&gt; marked &quot;review?&quot;.  This design avoids the need for additional flags or<br>
&gt; indications that a patch should be run through the queue.<br>
&gt;<br>
&gt; 4) &quot;Running tests on an arbitrary patches [opens a security hole].&quot;<br>
&gt; Instead of running check-webkit-style from it&#39;s own working copy, the<br>
&gt; style-queue runs check-webkit-style from a separate working copy.<br>
&gt; While it&#39;s true you could haxor my machine by committing an evil copy<br>
&gt; of check-webkit-style, I&#39;m already running that risk whenever I type<br>
&gt; &quot;svn up&quot;.<br>
&gt;<br>
&gt; == Frequently Asked Questions ==<br>
&gt;<br>
&gt; Q1: If the style-queue doesn&#39;t complain, does that mean my patch has<br>
&gt; correct style?<br>
&gt;<br>
&gt; A1: Unfortunately, no.  First of all, check-webkit-style has false<br>
&gt; negatives.  Hopefully, the script will improve over time, but it will<br>
&gt; never be perfect.  Second, the style-queue is only able to check<br>
&gt; patches that successfully download and apply to top-of-tree.  If your<br>
&gt; patch does not apply to top-of-tree (e.g., because it depends on<br>
&gt; another patch), then style-queue won&#39;t say anything.<br>
&gt;<br>
&gt; Q2: How can I see whether style-queue processed my patch?<br>
&gt;<br>
&gt; A2: There isn&#39;t a great way to do this yet.  The data is stored in the<br>
&gt; <a href="http://webkit-commit-queue.appspot.com" target="_blank">webkit-commit-queue.appspot.com</a> web service, but we haven&#39;t built a<br>
&gt; front-end for it yet.  If you have a strong stomach, you can look at<br>
&gt; the endpoint the bot itself uses<br>
&gt;<br>
&gt; <a href="http://webkit-commit-queue.appspot.com/patch-status/style-queue/12345" target="_blank">http://webkit-commit-queue.appspot.com/patch-status/style-queue/12345</a><br>
&gt;<br>
&gt; where 12345 is the ID of your attachment.  If you&#39;d like to have a<br>
&gt; better status display, feel free to hack on<br>
&gt; WebKitTools/QueueStatusServer and send me or Eric patches to review.<br>
&gt; In the long term, there&#39;s no reason we can&#39;t have an awesome UI.<br>
&gt;<br>
&gt; Q3: This bot is going to run amock!  How can I keep track of all it&#39;s<br>
&gt; evil deeds?<br>
&gt;<br>
&gt; A3: If you want to see what the bots are doing, you can subscribe to<br>
&gt; the following email list:<br>
&gt;<br>
&gt; <a href="http://groups.google.com/group/webkit-bot-watchers" target="_blank">http://groups.google.com/group/webkit-bot-watchers</a><br>
&gt;<br>
&gt; Whenever the bots touch a bug, they&#39;re supposed to add this address to<br>
&gt; the CC list.  This functionality is very new, so we might not have<br>
&gt; every case covered yet.  If you see the bot misbehaving, let me or<br>
&gt; Eric know.  We&#39;ll correct the issue ASAP.<br>
&gt;<br>
&gt; Q4: Checking style is fine and dandy, but I was hoping for a build-queue!<br>
&gt;<br>
&gt; A4: My next project is to get a build-queue working for Qt.  The<br>
&gt; build-queue will share a lot of infrastructure with the style-queue,<br>
&gt; but making the style-queue was easier because check-webkit-style is a<br>
&gt; lot faster than build-webkit.  :)<br>
&gt;<br>
&gt; Q5: When are you flipping the switch?<br>
&gt;<br>
&gt; A5: Either today or tomorrow, depending on when I find a stretch of<br>
&gt; time to babysit the queue through whatever initial troubles it has.<br>
&gt; My working copy of the style-queue is about five patches ahead of<br>
&gt; trunk.  It would be nice to have those patches committed before<br>
&gt; flipping the switch, but it&#39;s not necessary.<br>
&gt;<br>
&gt; Thanks for reading to the end, and I hope you find the style-queue useful.<br>
&gt;<br>
&gt; Adam<br>
&gt;<br>
</div></div></blockquote></div><br></div></div>