3219: Support "Unship It"

michaeldav**********@gmai***** (Google Code) (Is this you? Claim this profile.)
Feb. 6, 2014
What version are you running?

What's the URL of the page this enhancement relates to, if any?

Describe the enhancement and the motivation for it.
I need to be able to "Unship" (or Un-ship) reviews when I upload a new diff.

We require 2 "ship it!'s" before committing a chunk of reviewed code to trunk.  If reviewer X says Ship It! but then reviewer Y comes along and finds issues, I need to fix them, upload a new diff, and have reviewer X look again (could have made significant changes that would affect their original review).  

Similarly, one person saying Ship It twice should not increment the count of total ship its on the dashboard.

What operating system are you using? What browser?

All OSes, all browsers.

Please provide any additional information below.
#1 david
Fundamentally similar to bugs 489, 564, and 2106.
#2 chipx86
I want to address your comments on this though.

Different organizations have different workflows and different expectations for what the data means to them. It's perfectly reasonable in some places for a person to be able to say Ship It twice (each one may be for a different segment of the code), or for Ship Its to remain after a new diff (it's common to say "make these changes and Ship It!" and for the author to put up a new draft).

So we're unlikely to change any of that.

That said, RB is extensible, and it'd be easy to write an extension to clear existing Ship Its on new drafts. I'm sure some people would want that. Some wouldn't, which is why we probably won't bake it into the product without some serious thought.

We're looking to add some stuff to 2.0 for official "approval" support. It'd basically add support for extensions to specify whether a change is approved or not (for example, in your case, you could write a little bit of code requiring 2+ Ship It!'s and filter out dups). You could then have pre-commit hooks that check the flag, or whatever.