Rev 5227: (mbp) (mbp) mention babune in testing guide; in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed May 12 14:57:51 BST 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5227 [merge]
revision-id: pqm at pqm.ubuntu.com-20100512135749-8b0et7ntvh8dgm2t
parent: pqm at pqm.ubuntu.com-20100512095418-nsh69qc4r2nbnl35
parent: mbp at canonical.com-20100512110421-42amiixtpj4iskp5
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2010-05-12 14:57:49 +0100
message:
(mbp) (mbp) mention babune in testing guide;
split out review guide (Martin Pool)
added:
doc/developers/code-review.txt codereview.txt-20100512110229-wywsv1livg919f1f-1
modified:
doc/developers/HACKING.txt HACKING-20050805200004-2a5dc975d870f78c
doc/developers/index-plain.txt indexplain.txt-20090909123806-96yfsgrqwra8cwq7-1
doc/developers/index.txt index.txt-20070508041241-qznziunkg0nffhiw-1
doc/developers/testing.txt testing.txt-20080812140359-i70zzh6v2z7grqex-1
=== modified file 'doc/developers/HACKING.txt'
--- a/doc/developers/HACKING.txt 2010-04-16 12:59:03 +0000
+++ b/doc/developers/HACKING.txt 2010-05-12 11:04:21 +0000
@@ -179,11 +179,9 @@
* Bazaar - http://bazaar-vcs.org/
-* Bundle Buggy - http://bundlebuggy.aaronbentley.com/
-
* Patch Queue Manager - https://launchpad.net/pqm/
-For further information, see http://bazaar-vcs.org/BzrDevelopment.
+For further information, see <http://wiki.bazaar.canonical.com/BzrDevelopment>.
@@ -272,239 +270,6 @@
<http://doc.bazaar-vcs.org/developers/overview.html>`_.
-The Code Review Process
-#######################
-
-All code changes coming in to Bazaar are reviewed by someone else.
-Normally changes by core contributors are reviewed by one other core
-developer, and changes from other people are reviewed by two core
-developers. Use intelligent discretion if the patch is trivial.
-
-Good reviews do take time. They also regularly require a solid
-understanding of the overall code base. In practice, this means a small
-number of people often have a large review burden - with knowledge comes
-responsibility. No one likes their merge requests sitting in a queue going
-nowhere, so reviewing sooner rather than later is strongly encouraged.
-
-
-
-
-
-Review cover letters
-====================
-
-Please put a "cover letter" on your merge request explaining:
-
-* the reason **why** you're making this change
-
-* **how** this change achieves this purpose
-
-* anything else you may have fixed in passing
-
-* anything significant that you thought of doing, such as a more
- extensive fix or a different approach, but didn't or couldn't do now
-
-A good cover letter makes reviewers' lives easier because they can decide
-from the letter whether they agree with the purpose and approach, and then
-assess whether the patch actually does what the cover letter says.
-Explaining any "drive-by fixes" or roads not taken may also avoid queries
-from the reviewer. All in all this should give faster and better reviews.
-Sometimes writing the cover letter helps the submitter realize something
-else they need to do. The size of the cover letter should be proportional
-to the size and complexity of the patch.
-
-
-Reviewing proposed changes
-==========================
-
-Anyone is welcome to review code, and reply to the thread with their
-opinion or comments.
-
-The simplest way to review a proposed change is to just read the patch on
-the list or in Bundle Buggy. For more complex changes it may be useful
-to make a new working tree or branch from trunk, and merge the proposed
-change into it, so you can experiment with the code or look at a wider
-context.
-
-There are three main requirements for code to get in:
-
-* Doesn't reduce test coverage: if it adds new methods or commands,
- there should be tests for them. There is a good test framework
- and plenty of examples to crib from, but if you are having trouble
- working out how to test something feel free to post a draft patch
- and ask for help.
-
-* Doesn't reduce design clarity, such as by entangling objects
- we're trying to separate. This is mostly something the more
- experienced reviewers need to help check.
-
-* Improves bugs, features, speed, or code simplicity.
-
-Code that goes in should not degrade any of these aspects. Patches are
-welcome that only cleanup the code without changing the external
-behaviour. The core developers take care to keep the code quality high
-and understandable while recognising that perfect is sometimes the enemy
-of good.
-
-It is easy for reviews to make people notice other things which should be
-fixed but those things should not hold up the original fix being accepted.
-New things can easily be recorded in the Bug Tracker instead.
-
-It's normally much easier to review several smaller patches than one large
-one. You might want to use ``bzr-loom`` to maintain threads of related
-work, or submit a preparatory patch that will make your "real" change
-easier.
-
-
-Checklist for reviewers
-=======================
-
-* Do you understand what the code's doing and why?
-
-* Will it perform reasonably for large inputs, both in memory size and
- run time? Are there some scenarios where performance should be
- measured?
-
-* Is it tested, and are the tests at the right level? Are there both
- blackbox (command-line level) and API-oriented tests?
-
-* If this change will be visible to end users or API users, is it
- appropriately documented in NEWS?
-
-* Does it meet the coding standards below?
-
-* If it changes the user-visible behaviour, does it update the help
- strings and user documentation?
-
-* If it adds a new major concept or standard practice, does it update the
- developer documentation?
-
-* (your ideas here...)
-
-
-Reviews on Launchpad
-====================
-
-From May 2009 on, we prefer people to propose code reviews through
-Launchpad.
-
- * <https://launchpad.net/+tour/code-review>
-
- * <https://help.launchpad.net/Code/Review>
-
-Anyone can propose or comment on a merge proposal just by creating a
-Launchpad account.
-
-There are two ways to create a new merge proposal: through the web
-interface or by email.
-
-
-Proposing a merge through the web
----------------------------------
-
-To create the proposal through the web, first push your branch to Launchpad.
-For example, a branch dealing with documentation belonging to the Launchpad
-User mbp could be pushed as ::
-
- bzr push lp:~mbp/bzr/doc
-
-Then go to the branch's web page, which in this case would be
-<https://code.launchpad.net/~mbp/bzr/doc>. You can simplify this step by just
-running ::
-
- bzr lp-open
-
-You can then click "Propose for merging into another branch", and enter your
-cover letter (see above) into the web form. Typically you'll want to merge
-into ``~bzr/bzr/trunk`` which will be the default; you might also want to
-nominate merging into a release branch for a bug fix. There is the option to
-specify a specific reviewer or type of review, and you shouldn't normally
-change those.
-
-Submitting the form takes you to the new page about the merge proposal
-containing the diff of the changes, comments by interested people, and
-controls to comment or vote on the change.
-
-Proposing a merge by mail
--------------------------
-
-To propose a merge by mail, send a bundle to ``merge at code.launchpad.net``.
-
-You can generate a merge request like this::
-
- bzr send -o bug-1234.diff
-
-``bzr send`` can also send mail directly if you prefer; see the help.
-
-Reviewing changes
------------------
-
-From <https://code.launchpad.net/bzr/+activereviews> you can see all
-currently active reviews, and choose one to comment on. This page also
-shows proposals that are now approved and should be merged by someone with
-PQM access.
-
-
-Reviews through Bundle Buggy
-============================
-
-The Bundle Buggy tool used up to May 2009 is still available as a review
-mechanism.
-
-Sending patches for review
---------------------------
-
-If you'd like to propose a change, please post to the
-bazaar at lists.canonical.com list with a bundle, patch, or link to a
-branch. Put ``[PATCH]`` or ``[MERGE]`` in the subject so Bundle Buggy
-can pick it out, and explain the change in the email message text.
-Remember to update the NEWS file as part of your change if it makes any
-changes visible to users or plugin developers. Please include a diff
-against mainline if you're giving a link to a branch.
-
-You can generate a merge request like this::
-
- bzr send -o bug-1234.patch
-
-A ``.patch`` extension is recommended instead of .bundle as many mail clients
-will send the latter as a binary file.
-
-``bzr send`` can also send mail directly if you prefer; see the help.
-
-Please do **NOT** put [PATCH] or [MERGE] in the subject line if you don't
-want it to be merged. If you want comments from developers rather than
-to be merged, you can put ``[RFC]`` in the subject line.
-
-If this change addresses a bug, please put the bug number in the subject
-line too, in the form ``[#1]`` so that Bundle Buggy can recognize it.
-
-If the change is intended for a particular release mark that in the
-subject too, e.g. ``[1.6]``.
-Anyone can "vote" on the mailing list by expressing an opinion. Core
-developers can also vote using Bundle Buggy. Here are the voting codes and
-their explanations.
-
-:approve: Reviewer wants this submission merged.
-:tweak: Reviewer wants this submission merged with small changes. (No
- re-review required.)
-:abstain: Reviewer does not intend to vote on this patch.
-:resubmit: Please make changes and resubmit for review.
-:reject: Reviewer doesn't want this kind of change merged.
-:comment: Not really a vote. Reviewer just wants to comment, for now.
-
-If a change gets two approvals from core reviewers, and no rejections,
-then it's OK to come in. Any of the core developers can bring it into the
-bzr.dev trunk and backport it to maintenance branches if required. The
-Release Manager will merge the change into the branch for a pending
-release, if any. As a guideline, core developers usually merge their own
-changes and volunteer to merge other contributions if they were the second
-reviewer to agree to a change.
-
-To track the progress of proposed changes, use Bundle Buggy. See
-http://bundlebuggy.aaronbentley.com/help for a link to all the
-outstanding merge requests together with an explanation of the columns.
-Bundle Buggy will also mail you a link to track just your change.
-
Coding Style Guidelines
#######################
=== added file 'doc/developers/code-review.txt'
--- a/doc/developers/code-review.txt 1970-01-01 00:00:00 +0000
+++ b/doc/developers/code-review.txt 2010-05-12 11:04:21 +0000
@@ -0,0 +1,170 @@
+The Code Review Process
+#######################
+
+All code changes coming in to Bazaar are reviewed by someone else.
+Normally changes by core contributors are reviewed by one other core
+developer, and changes from other people are reviewed by two core
+developers. Use intelligent discretion if the patch is trivial.
+
+Good reviews do take time. They also regularly require a solid
+understanding of the overall code base. In practice, this means a small
+number of people often have a large review burden - with knowledge comes
+responsibility. No one likes their merge requests sitting in a queue going
+nowhere, so reviewing sooner rather than later is strongly encouraged.
+
+
+
+Review cover letters
+====================
+
+Please put a "cover letter" on your merge request explaining:
+
+* the reason **why** you're making this change
+
+* **how** this change achieves this purpose
+
+* anything else you may have fixed in passing
+
+* anything significant that you thought of doing, such as a more
+ extensive fix or a different approach, but didn't or couldn't do now
+
+A good cover letter makes reviewers' lives easier because they can decide
+from the letter whether they agree with the purpose and approach, and then
+assess whether the patch actually does what the cover letter says.
+Explaining any "drive-by fixes" or roads not taken may also avoid queries
+from the reviewer. All in all this should give faster and better reviews.
+Sometimes writing the cover letter helps the submitter realize something
+else they need to do. The size of the cover letter should be proportional
+to the size and complexity of the patch.
+
+
+Reviewing proposed changes
+==========================
+
+Anyone is welcome to review code, and reply to the thread with their
+opinion or comments.
+
+The simplest way to review a proposed change is to just read the patch on
+the list or in Bundle Buggy. For more complex changes it may be useful
+to make a new working tree or branch from trunk, and merge the proposed
+change into it, so you can experiment with the code or look at a wider
+context.
+
+There are three main requirements for code to get in:
+
+* Doesn't reduce test coverage: if it adds new methods or commands,
+ there should be tests for them. There is a good test framework
+ and plenty of examples to crib from, but if you are having trouble
+ working out how to test something feel free to post a draft patch
+ and ask for help.
+
+* Doesn't reduce design clarity, such as by entangling objects
+ we're trying to separate. This is mostly something the more
+ experienced reviewers need to help check.
+
+* Improves bugs, features, speed, or code simplicity.
+
+Code that goes in should not degrade any of these aspects. Patches are
+welcome that only cleanup the code without changing the external
+behaviour. The core developers take care to keep the code quality high
+and understandable while recognising that perfect is sometimes the enemy
+of good.
+
+It is easy for reviews to make people notice other things which should be
+fixed but those things should not hold up the original fix being accepted.
+New things can easily be recorded in the Bug Tracker instead.
+
+It's normally much easier to review several smaller patches than one large
+one. You might want to use ``bzr-loom`` to maintain threads of related
+work, or submit a preparatory patch that will make your "real" change
+easier.
+
+
+Checklist for reviewers
+=======================
+
+* Do you understand what the code's doing and why?
+
+* Will it perform reasonably for large inputs, both in memory size and
+ run time? Are there some scenarios where performance should be
+ measured?
+
+* Is it tested, and are the tests at the right level? Are there both
+ blackbox (command-line level) and API-oriented tests?
+
+* If this change will be visible to end users or API users, is it
+ appropriately documented in NEWS?
+
+* Does it meet the coding standards below?
+
+* If it changes the user-visible behaviour, does it update the help
+ strings and user documentation?
+
+* If it adds a new major concept or standard practice, does it update the
+ developer documentation?
+
+* (your ideas here...)
+
+
+Reviews on Launchpad
+====================
+
+From May 2009 on, we prefer people to propose code reviews through
+Launchpad.
+
+ * <https://launchpad.net/+tour/code-review>
+
+ * <https://help.launchpad.net/Code/Review>
+
+Anyone can propose or comment on a merge proposal just by creating a
+Launchpad account.
+
+There are two ways to create a new merge proposal: through the web
+interface or by email.
+
+
+Proposing a merge through the web
+---------------------------------
+
+To create the proposal through the web, first push your branch to Launchpad.
+For example, a branch dealing with documentation belonging to the Launchpad
+User mbp could be pushed as ::
+
+ bzr push lp:~mbp/bzr/doc
+
+Then go to the branch's web page, which in this case would be
+<https://code.launchpad.net/~mbp/bzr/doc>. You can simplify this step by just
+running ::
+
+ bzr lp-open
+
+You can then click "Propose for merging into another branch", and enter your
+cover letter (see above) into the web form. Typically you'll want to merge
+into ``~bzr/bzr/trunk`` which will be the default; you might also want to
+nominate merging into a release branch for a bug fix. There is the option to
+specify a specific reviewer or type of review, and you shouldn't normally
+change those.
+
+Submitting the form takes you to the new page about the merge proposal
+containing the diff of the changes, comments by interested people, and
+controls to comment or vote on the change.
+
+Proposing a merge by mail
+-------------------------
+
+To propose a merge by mail, send a bundle to ``merge at code.launchpad.net``.
+
+You can generate a merge request like this::
+
+ bzr send -o bug-1234.diff
+
+``bzr send`` can also send mail directly if you prefer; see the help.
+
+Reviewing changes
+-----------------
+
+From <https://code.launchpad.net/bzr/+activereviews> you can see all
+currently active reviews, and choose one to comment on. This page also
+shows proposals that are now approved and should be merged by someone with
+PQM access.
+
=== modified file 'doc/developers/index-plain.txt'
--- a/doc/developers/index-plain.txt 2010-02-23 04:12:26 +0000
+++ b/doc/developers/index-plain.txt 2010-05-12 11:04:21 +0000
@@ -24,6 +24,8 @@
* `Testing <testing.html>`_ |--| Guide to writing tests for Bazaar.
+* `Code Review <code-review.html>`_.
+
* `Writing plugins <http://doc.bazaar.canonical.com/plugins/en/plugin-development.html>`_
|--| specific advice on writing Bazaar plugins. (web link)
=== modified file 'doc/developers/index.txt'
--- a/doc/developers/index.txt 2010-02-23 04:12:26 +0000
+++ b/doc/developers/index.txt 2010-05-12 11:04:21 +0000
@@ -23,6 +23,7 @@
bug-handling
HACKING
testing
+ code-review
* `Contributing to Bazaar Documentation <http://wiki.bazaar.canonical.com/ContributingToTheDocs>`_ (wiki)
=== modified file 'doc/developers/testing.txt'
--- a/doc/developers/testing.txt 2010-05-06 23:41:35 +0000
+++ b/doc/developers/testing.txt 2010-05-12 10:51:28 +0000
@@ -6,7 +6,7 @@
The Importance of Testing
=========================
-Reliability is a critical success factor for any Version Control System.
+Reliability is a critical success factor for any version control system.
We want Bazaar to be highly reliable across multiple platforms while
evolving over time to meet the needs of its community.
@@ -183,6 +183,16 @@
.. _testrepository: https://launchpad.net/testrepository
+
+Babune continuous integration
+-----------------------------
+
+We have a Hudson continuous-integration system that automatically runs
+tests across various platforms. In the future we plan to add more
+combinations including testing plugins. See
+<http://babune.ladeuil.net:24842/>. (Babune = Bazaar Buildbot Network.)
+
+
Writing Tests
=============
More information about the bazaar-commits
mailing list