The criteria for marking patches as verified and for passing code review
are now documented at
http://beaker-project.org/dev-guide.html#reviewing-a-patch
We'd generally been applying these anyway, but now they're written down
so people don't need to guess about what we're looking for (and they're
a useful checklist for us as well).
===============
The "+1 Verified" marker indicates one of the following:
If it’s a bug fix that is reproduceable and testable, the new test
case has been verified to fail before the fix, and pass after the fix.
If it’s a bug fix that is not amenable to an automated test, the
patch has been verified to fix the bug through some other means (such as
trying it out manually).
If it’s a new feature, the feature has been verified to work as
described.
If it’s a code change, the test suite has been verified to pass in
full.
If it’s a docs change, the docs have been verified to build
correctly and look right.
On some rare occasions (for example, fixing a typo in a comment or
README), it may simply indicate that the patch has been determined not
to run a risk of breaking the application or documentation.
The "+2 Approved" code review marker should only be granted when all the
following criteria are met:
All significant review comments have been addressed, with the aim
of ensuring the Beaker code remains maintainable rather than
degenerating over time.
Whenever practical, automated tests have been added to ensure the
bug fix or new feature works as expected.
The code is commented appropriately (for example, explanations or
issue tracker references are included for any obscure workarounds).
The documentation (including docstrings) has been updated appropriately
A release note has been added as described in the What’s New source
for new features, bug fixes that may break existing workarounds, and any
changes that require manual steps from system administrators when
upgrading an existing installation.
The commit message is correctly formatted with a short summary line
and any additional continuation lines separated from the summary by a
blank line.
For changes driven by a Bugzilla entry, the correct "Bug: NNNNNN"
reference is present in the commit message (as described above in
"Submitting your patch").
===============
Cheers,
Nick.
--
Nick Coghlan
Red Hat Infrastructure Engineering & Development, Brisbane
Test Automation Team Lead
Beaker Development Lead (
http://beaker-project.org/)
PulpDist Development Lead (
http://pulpdist.readthedocs.org)