Toshio,
Thanks for taking the time to look it over.
>
> Architectural Comments:
> * I like that you've split out blocker criteria from non-blocker
> criteria. I would split security out too so the new reviewer can
> realize this list is _really_ important. I purposefully didn't
examine
> what was in your lists of critical vs non-critical because I think we
> need to discuss what standards we as a group want our packages to be
> held up to. I've been going on the assumption that everything on the
> QAChecklist is mandatory. If that's not so, it has to be discussed
(and
> perhaps voted) as it's our collective name as a quality repository
> that's at stake. [We might be seeking to make a bigger repository,
but
> we still have to balance that against quality.]
Agreed. My initial impression was that the QACheckList was mandatory
too. However, I quickly found out that several of those items are
subject to a LOT of contention, and are generally NOT considered
showstoppers.
I think unless there is some sort of unanimity about a given item, it
has to be considered a non-showstopper. The alternative is to not accept
packages or input from those who disagree, which is partially why we
have as many "non-standard" repositories as we do.
This is a subject for much discussion, I agree. I've tried to put
something down for people to argue over and be proactive instead of
whining.
>
> * I don't like the fact that the QAChecklist contains most of the
things
> in this document but is a separate document. I think the QAChecklist
> needs to be tuned up with categories of blocker, non-blocker. And
> perhaps in terms of order of doing things. And then it can be
> referenced from this page. Having another page with overlapping
> Checklists adds to the confusion of "which list do I follow?" and
means
> policy changes have to be updated in two places.
>
Agreed here as well. I'd like to see the QACheckList be the definitive
list of blockers and non-blockers. Right now it's a hodge podge of
how-to's, critical checks, and opinions.
My guide is meant as a one-stop shop to understanding the QA checklist
and its accompanying documents. I don't see it being a definitive list
of things to check, or necessarily of fedora.us QA policy.
Personally, I'd like to see my document become the official "high level
overview" of the policy, and the qa checklist become the definitive list
of policy, at a detailed level.
> Editorial comments:
> [3.1] - I remember having to add fedora.us to my yum.conf manually but
> I'm not sure if that was before or after I installed FC1. If FC1
> doesn't include it, it needs to be mentioned.
> - mach needs apt installed as well.
Yes. However, I stated that I assumed the QA volunteer had set up their
system to use fedora.us already. Mach installs apt as a dependency, and
I'm working with Thomas to get my changes merged into the upstream mach.
They should be available via fedora.us by the time this document goes
live.
I've changed the document to assume that the user can install mach from
fedora.us.
> [3.2] - I like gpasswd more than vigr because `gpasswd -a [USERNAME]
> mach` is a more complete explanation than edit groups with vigr.
Good suggestion. I forgot about gpasswd. Change made.
> [3.x] - There needs to be something in here about modifying
~/.rpmmacros
> to point to another build directory as /usr/src/redhat is FHS and
> root-user problematic.
This was another assumption I made up above. I think we'll need an
accompanying document (maybe there is a good one we can just link in?)
that will explain how to setup and use rpm out of one's home directory.
I agree we DO NOT want people building out of /usr/src/redhat.
I don't really know how many people know how to do this. As I see it, if
they don't know how, they've got a lot to learn before they can do QA
effectively.
> [4] - I'd add some note to pay attention to see what good and bad
things
> to look for. How things are structured into Critical and non-critical
> comments. Reviews should include an SRPM MD5sum _at_minimum_.
My belief is that reviews should contain ALL the things I have in the
template. That includes the SRPM filename and MD5Sum, and the MD5Sums
for the source packages. The non-critical comments should go in the
notes section of my template.
I don't see much point in QA without some accountability that says "Yes,
I checked the basics, and the package seems ok". I realize not everyone
agrees with me here.
A standard format also has the benefit of being able to be automated.
For instance, one could write a script to troll bugzilla comments for
reviews and validate them for completeness, check md5sums, or whatever.
> [6] - Not all packages have MD5sums. And the improtant part of this
> step is to correlate a package to a gpg key. Either MD5sums are
> provided and signed with a gpg key (which needs to be gpg verified) or
> the package itself needs to be rpm --checksig verified. The Fedora.us
> policy says that all packages need to be rpm --addsign'ed so you can
> leave out the MD5Sum verification if you're feeling lazy.
My explanations of verifying package integrity obviously need some work.
As I see it there are two aspects.
1. Are you checking the same package the author uploaded? Clearsigned
md5sums or gpg sigs on the SRPM should do this.
This is part of step 5, but should be elaborated on.
How do you easily get somebody's gpg key on your keyring? One of the
things that slowed me down a lot was attempting to track down peoples
public keys.
I've changed the document to require checking with rpm --checksig in
addition to checking md5sums.
2. Is the author using legimitate sources. Verifying md5sums or gpg sigs
on the tarballs and patches in the SRPM is the only way to do this. This
part is the non-obvious one, because it's tough to find the sources the
author used sometimes.
I've attempted to make this more clear... Please review
> [8] - Note to google. Note to _not_trust_the_spec_ completely. It's
> important that the reviewer verify that the URL's and etc are not
> leading to false download sources.
Good point. I've added a note. Please review.
> [14] - A use your judgement about additional things should be added.
> (And where to ask QA/Packaging questions -- irc #fedora-devel, mailing
> lists, asking on the bugzilla, etc)
> - Other keywords need mentioning. PUBLISH, NEEDSWORK, QA come to
> mind. REVIEWED only applies if no one else has already REVIEWED it.
>
> [Example Review Template]
> - There needs to be an example of a negative review as well.
Hmm. I've added one. See what you think.
> - GPG signing is _important_. It's not even mentioned.
Yes it is, in brief. For a newbie I don't believe it's critical since
their reviews will be somewhat suspect anyway.
> - I like MD5Sums of all files in the SRPM.
> - It's better to have the output of md5sum verbatim in the review.
> Easier to automate verification than having it split over multiple
> lines.
Good idea. See changes in document.
> It's a good start on a helpful document.
Thanks, and thanks for the input.
--erik