Thanks for your feedback. Comments inline.
On Tue, Oct 6, 2009 at 8:17 AM, Biking4Fun <govo0...@gmail.com> wrote:
> Only show requests that need your action - Tennis Ball model
> Currently, a review stays in everyone's view until the requester has
> marked it submitted, etc. This tends to clutter one's view and makes
> it unclear what you need to act upon.
> It would be better if ReviewBoard implemented a tennis / ping pong
> What I mean by that is the review should only be in someone's view
> when they need to act on the review. Once the reviewer has commented
> and/or marked it "Ship It", that is like hitting the ball back to the
> requester's court. It should no longer appear in that particular
> reviewer's view, because they have already acted upon it. It should
> now show up in the requester's view, because someone has commented and
> perhaps found a problem.
There's a lot of value in this model, but we'd have to be careful with how
we implement it. It's not always ideal to only see review requests you
haven't commented on. There are times when I've only been able to review
part of a very large change, made that review public, and then went back to
it later to review more of it. In cases like these, I'd certainly want to
see the review request still.
I think what we could do is add a form of filtering to the dashboard, so you
could show or hide review requests you've already looked at that haven't
been updated since you last saw it.
I don't know that anything needs to be done in the requester's view. A
person generally only has a few review requests out at any given point in
time. The above filter could just apply to this view as well if it's really
Feel free to file a feature request on this, so we can track it for future
> Some code changes are for future releases, a lower priority. Others
> are for a critical fix, or trying to make a code freeze deadline- a
> higher priority. There currently isn't a way for a requester to
> indicate that a particular review is important and needs to be looked
> at quickly.
We've discussed adding some sort of severity indicator to a review request.
Maybe we will eventually, but we're not set on it yet for a couple of
1) Severity levels can (and will) be abused. Everyone thinks their review
request is important and unless you're working with Git or something
similar, you're often blocked until your change goes in. If we give the
ability to indicate low, medium and high priorities, then most people will
probably pick high, in order to get their change looked at. Review requests
with lower priorities will probably just end up being ignored, as there will
always be higher priority review requests. In the end, this won't do anyone
2) Severity levels set by the person posting the review request makes sense
in corporate environments but often don't in open source projects. If
someone is submitting a patch to Review Board, for instance, I really don't
want them to decide that it's critical to get it in. It makes a lot more
sense for myself as the developer to decide the priority of it. But then,
that doesn't really help us any, because if we consider it critical to get
in now, we should just review it now.
3) Severity levels don't communicate *why* it's important. Is it because
there's a beta coming up that needs that change? Is it blocking the person?
Is it blocking somebody else? Is it some emergency security fix? Build
breakage? I don't think a severity level communicates enough here.
4) There's already a mechanism for communicating that something is important
and needs to be done for some deadline or milestone. Put it in the summary.
At VMware, for example, people will sometimes prefix a summary with "[Beta 1
fix]" and everyone working on the product will know that beta 1 is coming up
and that that needs to take priority. No need for any special fields in the
database, special UI, or anything.
We may reconsider it sometime, but our design philosophy so far has been to
keep things pretty straightforward and free of a lot of policy. Many things
can be communicated quite well through text, and introducing special UI just
means that users now have to learn one more little thing. It also often
means that, while the feature is good for some people, it won't meet someone
else's need/preference, and we'll be asked to expand on it, thus making it
more complicated for everyone else. So far, we've worked to keep the number
of features like this in the product pretty small.
Hope that made sense.
Christian Hammond - chip...@chipx86.com
Review Board - http://www.review-board.org
VMware, Inc. - http://www.vmware.com
You received this message because you are subscribed to the Google Groups
To post to this group, send email to firstname.lastname@example.org
To unsubscribe from this group, send email to
For more options, visit this group at