On Sat, 30 Dec 2006 13:28:10 -0500 jkeating@xxxxxxxxxx (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
Attachment:
signature.asc
Description: PGP signature
-- Fedora-maintainers mailing list Fedora-maintainers@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-maintainers
-- Fedora-maintainers-readonly mailing list Fedora-maintainers-readonly@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-maintainers-readonly
