RFC: Review with Flags (Version 3)
Hans de Goede
j.w.r.degoede at hhs.nl
Wed Feb 7 08:08:42 UTC 2007
Warren Togami wrote:
First of all let me say that I'm very happy that this process is still
being worked on / streamlined!
> Raw notes... your comments would be very much appreciated.
>
> Key Changes from Previous Procedures
> ====================================
> 1) Assigned to pointer indicates that there is a reviewer. Assigned
> pointer to nobody means there is no reviewer yet.
> 2) BLANK means review not requested. ? means review requested. This
> makes it easier to query, because querying for BLANK is a bit inefficient.
> (Good idea, bad idea?)
> 3) NEEDINFO is used to avoid changing the Assigned pointer when fixes
> are needed.
> 4) Assigned pointer DOES NOT CHANGE during and after the review.
>
> Fedora Review Flag States
> =========================
> fedora-review BLANK (Not under review at all!)
> fedora-review? (I want a review)
> fedora-review- (rejected, needs work, set NEEDINFO to person who needs
> to fix it)
> fedora-review+ (APPROVED)
>
Why all the ping-ponging between ? and - during the review and all the
ping-ponging between ASSIGNED and NEEDINFO. In FE we've never done this
ping-ponging and we've never felt a need to introduce this IMHO this are
just unnecesarry mouse clicks. NEEDINFO and fedora-review- may be a good
idea if either the person requesting the review or the reviewer are slow
to respond. But in my experience there is a bit of quick discussion
between the two and the whole review process is finished with a day or
two (for normal packages) I really feel that all this changing of flags
and status is just unnecesarry mouse clicks.
> Assigned Pointer
> ================
> (Note, Assigned pointer is different from the ASSIGNED state.)
> - Assigned pointer to nobody at fedoraproject.org if no reviewer yet.
> - Assigned pointer to reviewer, during and after review.
> - Set Assigned pointer back to nobody if reviewer gave up.
>
Good.
> Bugzilla States
> ===============
> NEW ASSIGNED REOPENED (rather meaningless distinction?)
> NEEDINFO (to owner or somebody who needs to act to fix)
> MODIFIED (seems to be fixed, please test it)
> CLOSED (ticket is done)
>
> Process
> =======
> 1. Review Request is filed.
> 2. Set fedora-review? flag to signify that the ticket is ready for review.
Shouldn't the review request form also set this flag for us, I see no
gain in having todo this manually.
> 3. Reviewer takes bug, Assigned pointer set to self.
> 4. If APPROVED, fedora-review+.
> 5. If rejected, fedora-review- and NEEDINFO owner to fix it.
What is rejected? See above, often there are a couple of small must FIX
/ should fix items, which usually get fixed really soon.
> 6. If fedora-review- and owner fixes something, set back to ? to request
> review again.
Ping-pong-ping-pong, /me no like
> 7. After fedora-review+, initiate the fedora-cvs request procedure.
> 8. After fedora-cvs, checkin, build, and set to CLOSED.
>
> Drawbacks
> =========
> Since the mass review tickets were not filed by the package owners, it
> is less than obvious. Most reviews however are filed by owners, so this
> is less of a problem beyond this mass review.
>
to much ping-pong
> Other Possibilities
> ===================
> fedora-review? could also be used on any other Fedora bug when a
> horrible mess is found in an existing package, and attention for a
> re-review would be desired to fix it. (Good idea, bad idea?)
>
In general I like being able to say this is a mess, this needs a
re-review, but we need some rules for this, like whom may initiate this?
> Possible Process Optimizations
> ==============================
> (These might possibly help... although they might require hacks in
> Bugzilla's code, so we only request these optimizations only if they
> would make a huge positive difference. Your thoughts?)
> 1. Changing fedora-review in any way should auto-CC yourself automatically.
Not necessary with the new assigned rules, the review requester is the
reporter and thus get mails, the reviewer is the assignee and thus gets
mail.
> 2. Changing fedora-review to + should auto-set Assigned pointer to self.
>
? You write above "Assigned pointer to reviewer, during and after
review." notice the and after, thus this also isn't needed.
Regards,
Hans
More information about the Fedora-maintainers
mailing list