345145 - Change Sunbird review flags

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Administration, task)

Description

[Joey Minta]

Currently Sunbird has first-review and second-review flags for patches.  We'd like these to be removed and replaced with review and owner-review.  The review flag should be able to be set multiple times, as well.  Previously set first and second-review flags should both be mapped to the new review flag.  Talking with myk, he seemed to think this could all be done without too much trouble, but if there are implementation problems, we can rethink this scenario and adjust accordingly.

Comment 1

[timeless]

the general review flag is multiplicably requestable. i think you probably don't want your old flag removed, just disabled, otherwise you lose your old reviews.

as for owner review. we don't have such a thing right now, and i'm too tired to think about trying to add it. experience has taught me that if i don't get things right the first time, i'm forever haunted. certainly the review flag is haunting me to this day.

Comment 2

[timeless]

oh man. you explictily said 'previously set flags should be mapped'

either you weren't clear or myk wasn't thinking about what you said.

no, we can't do this. note that both first-review and second-review flags are *shared* with other components. so we can't just rename them.

Comment 3

[Joey Minta]

(In reply to comment #2)
> oh man. you explictily said 'previously set flags should be mapped'
> 
> either you weren't clear or myk wasn't thinking about what you said.
myk seemed to think this was possible via some SQL fun.  Specifically:
16:16 < myk> lilmatt: so, as i recall the SQL fu isn't particularly 
16:16 < myk> it's basically just rerooting instances of flags from one class to              another (i.e. update flags set flag_type = <new id> where 
             flag_type = <old id> and product_id = <calendar product id>)

16:21 < myk> so, my suggestion would be to file the bug requesting what you 
             want, then consider workarounds if it seems like the server admins 
             can't get to it in a timely fashion
16:22 < myk> you'll want to specify which existing flags should be migrated to 
             which new flags, i.e. first-review -> review, second-review -> 
             owner-review (or review, if most second-reviews aren't actually 
             owner-reviews)
16:22 < myk> and specify that you want review to be multiplicable (which gives 
             you the addl. review), but you don't want owner-review to be 
             multiplicable

(In reply to comment #1)
> the general review flag is multiplicably requestable. i think you probably
> don't want your old flag removed, just disabled, otherwise you lose your old
> reviews.
> 
> as for owner review. we don't have such a thing right now, and i'm too tired to
> think about trying to add it. experience has taught me that if i don't get
> things right the first time, i'm forever haunted. certainly the review flag is
> haunting me to this day.
> 
Does that mean today, when you're more awake, this can happen? Or that we need another approach?

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

Ehm, do we really need this? What's wrong with good-old first-review and second-review flags? Sure, the proposed names might be slightly clearer, but would give a lot of mess (like timeless mentioned).
Let's try the whole new review process first, before messing in bugzilla.

Comment 5

[Joey Minta]

(In reply to comment #4)
> Ehm, do we really need this? What's wrong with good-old first-review and
> second-review flags? Sure, the proposed names might be slightly clearer, but
> would give a lot of mess (like timeless mentioned).
> Let's try the whole new review process first, before messing in bugzilla.
> 
So the idea here is that we're running into cases where we simply don't have enough review flags for patches.  Furthermore, the simple solution of making the flag multiplicable wasn't available, since first-review is shared across other products. Therefore, myk suggested that we take this approach.  Renaming the flags isn't nearly as high of a priority, but since we were already changing the flags, it made sense to clarify their names at the same time.

Comment 6

[Martin Schröder [:mschroeder]]

Can this bug be closed? The review process has changed from first/second review to a single review.

Comment 7

[timeless]

yes, i did this a while ago

Comment 8

[Reed Loden [:reed]]

Closing, as of comment #6 and comment #7.