[webkit-dev] style-queue entering beta

[Eric Seidel]

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 <abarth at webkit.org> 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 <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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20091130/7dc66e92/attachment.html>