Summary from yesterdays (mini) FESCo meeting

[Kevin Fenzi]

On Sat, 30 Dec 2006 13:28:10 -0500
jkeating at redhat.com (Jesse Keating) wrote:

> On Saturday 30 December 2006 13:15, Axel Thimm wrote:
> > Well, in the same hyperbolic nature an "APPROVED" only says that a
> > person hit eight characters on his keyboard in the proper sequence.
> >
> > We should stop assuming the worse from the reviewers and just guide
> > them to do their review properly. And even if you do assume bad
> > reviewer then the checklisting is the way to find what they missed
> > and why.
> >
> > Let's try another approach: Other than the people having written the
> > review guidelines no one has memorized the list (probably the
> > authors didn't either) and will have to check the MUSTs somewhere,
> > be it in the wiki or his personal notes. If he is going to check
> > the items why not publicly in the bugzilla? It's zero effort in
> > addition, unless the reviewer didn't check the MUSTs at all, and
> > that would be bad and worth catching.
> 
> Having a checklist is fine, forcing reviewers to paste it into a
> review bug purely so that somebody could have a warm and fuzzy
> feeling that the review was actually done is silly.  So sure,
> maintain a condensed checklist of the musts/shoulds.  Review
> guidelines point to this.  Forcing it to be pasted into bugs, not so
> much

I would have to agree with Jesse here... 

I use a checklist, but because it's useful for me to remember to check
through everything. If someone is able to check all the must items from
memory or notice when they are out of place, more power to them... 
I don't think one should be required. 

For folks that don't like the 'APPROVED' type reviews, how about
re-reviewing them and looking to see if anything was missed? If it was,
point out to the reviewer that they need to make sure and check the
item(s) that they missed, and perhaps that would even drive them to
start using a checklist moving forward. 

One other thing to take into account is the history of the person doing
the review. If I saw a review done by say mschwent or tibbs that just
said 'APPROVED', I would suspect they looked over the package fully.
If I saw that from a new first time reviewer, I would want to run a
re-review sanity check to make sure they checked over everything they
should have.  

If we had a big pool of reviewers I could even see doing things like
random re-reviews of packages (draw a random package out of the hat and
re-review it and file bugs for any problems found), or having multiple
reviewers per submission (so multiple eyes might find something one
pair would miss), or re-review packages that have updates often, and so
on. Unfortunately, we don't have a big pool of reviewers, so we do the
best we can... 

If someone in the existing pool of reviewers is doing a poor job,
perhaps their fedorabugs membership could be removed? If more poor
reviews creep in, perhaps getting fedorabugs membership could be
tightened? 

In any case I don't think a requirement for what a reviewer must say in
a package review request ticket is a good idea. 

Oh well, wasted enough time reading and replying to this thread... 
off to do some reviews. ;) 

kevin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/fedora-maintainers/attachments/20061230/a73c7973/attachment.sig>