RFC: Review with Flags (Version 5)

[Hans de Goede]

Warren Togami wrote:
> This procedure is meant to be for *BOTH* Merge and regular package 
> reviews.  Please comment.  I hope we can finally ratify something during 
> this Thursday's FESCO meeting.
> 
> Changes since Version 4:
> ========================
> - ASSIGNED to nobody if there is no reviewer yet.
> - ASSIGNED remains on reviewer thereafter.
> - Use NEEDINFO if someone other than the reviewer needs to take action.
> 
> Fedora Review Flag States
> =========================
> fedora-review BLANK
>     I want a review, or a past reviewer gave up.
> fedora-review?
>     Under Review, ASSIGNED to reviewer
> fedora-review-
>     Denied and needs work, NEEDINFO to owner
> fedora-review+
>     APPROVED, ASSIGNED to reviewer
> 
> Assigned Pointer
> ================
> - Assigned pointer to nobody at fedoraproject.org if no reviewer yet.
> - Assigned pointer to reviewer, during and after the review.
> 
> Bugzilla States
> ===============
> In practice a bug sitting in these states matter less than the state of
> the fedora-review flag.  Participants are to follow these states as
> suggested guidelines, but the fedora-review flag has the hard
> requirements of behavior.
> 
> NEW ASSIGNED REOPENED
> - There is no real distinction between these states, they all generally 
> mean "open".
> 
> NEEDINFO
> - To owner or other person who needs to fix something or provide needed
> information in order for the review to proceed.
> 
> MODIFIED
> - Owner seems to have fixed it, but it requires testing.
> - OPTIONAL: you don't need to use this state.  It could sit in ASSIGNED
> where you do the same thing.  This might seem confusing, but we can't 
> stop people from using this state.  Yet another thing to simplify away 
> in the future ideal process.
> - *Special Case: During the Mass Review, the fix may go into rawhide and
> the reviewer can verify both the CVS contents and package before giving
> fedora-review+.
> 
why is this one optional but need info not? To me
fedora-review- and NEEDINFO should only be used if there is a dispute / 
things are moving slow, normally a review can have quite a few quickly 
following eachother comments where the reviewer and submitter work 
things out I still see no use in this PING PONG-ing of the status and 
fedora-review flag, this has been mentioned by me and several others 
several times, why is no one listening or atleast a response is geiven 
explaining what the perceived advantage of using NEEDINFO is, even if 
the review is going smoothly. Even better make the NEEDINFO 
fedora-review- step optional.

To me fedora-review- should be used with CLOSED WONTFIX if the package 
is denied forever (for example if the license is no good) and NEEDINFO 
should be used as it always is in bugzilla.

> Review Process
> ==============
> 1. Review Request is filed
>     fedora-review is BLANK
>     Assigned to nobody
> 2. Reviewer Takes a Request
>     fedora-review is ?
>     Assigned to reviewer

> 3a. If review denied and needs work
>     Comment
>     fedora-review-
>     NEEDINFO to whoever needs to fix it.
> 3b. fedora-review- and owner provides fix
>     fedora-review back to ?, to request re-review
Can we PLEASE make step 3a and 3b optional, I only see extra mouse 
clicks with little added value here.

> 4. If APPROVED
>     fedora-review+
> 5. After fedora-review+
>     initiate the fedora-cvs request procedure
> 6. After fedora-cvs procedure (empty directories are in CVS)
>     checkin
>     build
>     verify buids
>     set to CLOSED RAWHIDE
> 

Regards,

Hans