RFC: Review with Flags (Version 4)

[Dominik 'Rathann' Mierzejewski]

On Thursday, 08 February 2007 at 05:55, Warren Togami wrote:
> I think this procedure should be good enough for both Mass Review and 
> general package review for an interim period, prior to a better design 
> in Package Database.  I would like to ratify this process late Thursday 
> if possible, so please comment soon if you see problems.
> 
> Changes since Version 3:
> ========================
> - Hybrid of "ASSIGNED to next actor" and "ASSIGNED to reviewer and use 
> NEEDINFO" as summarized in 
> https://www.redhat.com/archives/fedora-maintainers/2007-February/msg00252.html
> - Explicit description of MODIFIED and CLOSED states
> 
> 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

No point in flipping the flag during review, NEEDINFO is enough.

> fedora-review+
> 	APPROVED, ASSIGNED to owner

Please let it stay assigned to the reviewer. How do I get a list of packages
I reviewed in the past otherwise?

> 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 the review.
> - Assigned pointer to nobody if reviewer gave up.
> - Assigned pointer to owner, after APPROVED and fedora-review+.

See above.

> 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.  The flag and 
> Assigned to pointer is what matters.
> - Note that ASSIGNED state is different from the Assigned pointer and 
> has no apparent relation for our purposes.
> 
> NEEDINFO
> - To owner or other person who needs to fix something or provide needed 
> information in order to proceed further.
> 
> 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.

I vote for staying with ASSIGNED.

> - *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+.
> 
> CLOSED RAWHIDE
> - fedora-review+ is APPROVED, CVS procedure is done, and package is 
> built and confirmed to be done.
> - *Special Case*: During the Mass Review, it is fine to set to CLOSED 
> RAWHIDE if it is confirmed to be fixed there.  Please use MODIFIED prior 
> to CLOSED RAWHIDE to allow for a verification step.
> 
> 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
> 4. If APPROVED
> 	fedora-review+
> 	Assign to owner
> 5. After fedora-review+
> 	initiate the fedora-cvs request procedure
> 6. After fedora-cvs procedure
> 	checkin
> 	build
> 	verify buids
> 	set to CLOSED RAWHIDE
> 
> 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?)

Good idea IMHO.

> Possible Process Optimizations
> ==============================
> 1. Changing fedora-review to ? auto-sets Assigned pointer to self.  This 
> is taking the review.

Yes.

> 2. Changing fedora-review to + should auto-set Assigned pointer to 
> owner.  This is a little more difficult because it isn't always obvious 
> who the owner is (especially in Mass Reviews), but this may be the 
> reporter in regular reviews later.

No. Leave the review assigned to the reviewer.

Regards,
R.

-- 
Fedora Extras contributor  http://fedoraproject.org/wiki/DominikMierzejewski
Livna contributor http://rpm.livna.org MPlayer developer http://mplayerhq.hu
"Faith manages."
        -- Delenn to Lennier in Babylon 5:"Confessions and Lamentations"