28.12.09

Review queue zero

Tags: , , , , — Jeff @ 16:00

I have zero outstanding review requests against me, placing me in a state in which I have not been since at least the first few months of the year. Yay! If you have review requests to make, and they could reasonably be addressed to me, feel free to do so. No one else is further ahead in that game than I am — for the moment. Carpe diem!

4 Comments »

  1. Thanks for the r-!

    Yes, the r-, because it’s well-reasoned and makes sense. I’ve always felt that having speedy reviews (+ or -) greatly motivates me to actually fix things up and try to submit more patches, whereas stale ones just mean that, by the time they eventually happen, I have lost all context and excuse (to people who own my time) to actually try to push things. I wish more people (including me) would try to drive their review queues towards zero :)

    Mozilla isn’t the only project that this happens to, of course – but that doesn’t mean I can’t keep wishing ;)

    Comment by Mook — 28.12.09 @ 23:43

  2. Yeah, apologies again for taking so long on that one.

    I’ve been making an effort for a couple months to get relatively new incoming reviews done in somewhere less than two weeks (your and one other for xpcshell testing were the big old ones, for a very long time, in this), in batches, excepting the occasional one-liner that I see and review instantly. I’ve found this particularly important because I’ve reviewed a handful of patches from newer faces where there may be less flexibility in timing — if you’ve been around longer, you probably have more things you want to fix, so a delay in one place still leaves many others where you can progress. Experience also breeds empathy, to a degree. (I stand in awe of people who really stay on top of their queues regularly, even given far greater review loads than I have.)

    Eventually, however, every review has to happen. Yours was complicated by parallel, semi-diverging development, plus some uncertainty about exactly how much extra work I could reasonably demand. Refactoring like what I mentioned is a task in which I would enjoy a comparative advantage, so it makes sense for me to do it — except that I have other tasks as well (ES5 work, mostly) that have higher priority with similar comparative advantage. It’s a difficult tradeoff between my working on high-priority things and working on things that block others. I just wish I’d noticed and comprehended the importance of the prefix-loop earlier, because that comment was half-written mentally months ago. Without taking notice of its crucial performance implications, however, I couldn’t quite bring myself to demand that refactoring in or as a precondition to your patch; it seemed more a code hygiene issue than an actual blocker until I thought harder about what the loop implied.

    (For those lacking context, we refer to bug 485255.)

    Comment by Jeff — 29.12.09 @ 01:56

  3. I can’t really judge the significance of your achievement without knowing which module(s) you actually have review powers for…

    Comment by Neil — 29.12.09 @ 04:39

  4. It’s not, really, except that it’s been so rare in recent times.

    Comment by Jeff — 29.12.09 @ 11:43

RSS feed for comments on this post. TrackBack URI

Leave a comment

HTML: You can use these tags: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>