[webkit-dev] style-queue entering beta

[Adam Barth]

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 <abarth at webkit.org> 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 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
> flipping the switch, but it's not necessary.
>
> Thanks for reading to the end, and I hope you find the style-queue useful.
>
> Adam
>