-
-
/trunk/reviewboard/reviews/datagrids.py (Diff revision 1) Have you tested what happens if the number of reviewers is greater than num_shipit? Looks like in that case you don't get the green checkmark, but something like 5/3. Is that what you want?
-
Yes. I found that bug and another one. I've been working on it for way too long, because I don't know how to do things in Python. =) The other bug is if someone who isn't in the reviewers list says "ship it", its gets counted too. The upcoming patch will address both of these bugs.
-
I really like your idea-- so much so that we've now done something similar with our installation of ReviewBoard based on your submission. But we had a different concept for what earns a green checkmark. For us, we want everybody that reviews the document to be unanimous that it should be approved, whether they are explicitly listed or not. But I can see that in many cases your way would be appropriate, and in the end we may adopt your approach after we try this out for a while, and after we start to use automatically assigned reviewers more.
-
My concern about this centers around the policy decisions, though. Not every group would want these numbers counted the same way (some want a fixed number of "Ship Its" before showing that it's ready to ship, some want a percentage, some are fine with one, etc.). I'm trying to keep changes that deal with policy like this to a minimum for now, but after 1.0 we want to come up with a good strategy for policy. This would fit in a lot better then. I think groups that really want this functionality should add it to their installs (though that'll be harder to maintain during upgrades), and we should revisit this down the road. I'd also probably be more comfortable if this was a brand new column instead of replacing/augmenting the current Ship It column. Maybe a "Number of Ship Its" column or something.
-
I suppose one way to handle this is to add each of these "policy decisions" into individual columns. There can be one for the original "Ship it!", one for my version, and one for Kimon's version. That way, people can reap the benefits of the new functionality, deciding how they prefer to use it without changing or omitting existing functionality. Also, we can submit this and get it into the code base so Kimon and I (and whoever else uses the patch) don't have to manually patch every time we upgrade!
-
The more I think about it, the more I'd like this to be a new column. That way existing behavior doesn't change at all. After 1.0, when we have extensions and policy, we can address this in a better way. At the very least, it will be possible for an extension to offer custom columns, so it'll be easier for you two to maintain your own more specific versions. This should make use of the new shipit_count variable on the ReviewRequest being introduced in /r/817/
-
-
Changes "ship-it!" status to "number of ship it"
Review Request #780 — Created March 24, 2009 and discarded
If no individual people are assigned to the review, "Ship It!" column function remains unchanged. If individual people are assigned to the review, it will return the fraction of "Ship It!" statuses achieved. IE "1/3" If 100% say ship it, the green checkmark image is displayed.
Works for me in all but one case. If a user submits 2 "ship it" reviews, it counts those reviews twice. How can you perform a filter, indicating "user" must be unique? (Sorry, rather new to Python here.)
LO
- Change Summary:
-
Addresses the two issues mentioned above. This certainly is not a pretty way to accomplish the task. However, I don't know how else to run a filter based upon a set/list of allowable values. I welcome anyone else's ideas if they can implement it more cleanly.
- Diff:
-
Revision 2 (+22 -4)