Bazaar

Merge lp:~mbp/bzr/doc into lp:bzr

  1. doc
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: 5227
Proposed branch: lp:~mbp/bzr/doc
Merge into: lp:bzr
Diff against target: 485 lines (+185/-237)
5 files modified
doc/developers/HACKING.txt (+1/-236)
doc/developers/code-review.txt (+170/-0)
doc/developers/index-plain.txt (+2/-0)
doc/developers/index.txt (+1/-0)
doc/developers/testing.txt (+11/-1)
To merge this branch: bzr merge lp:~mbp/bzr/doc
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+25137@code.launchpad.net

Commit message

(mbp) mention babune in testing guide; split out review guide

Description of the change

Mention Babune in the testing guide.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/doc into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #398653 Releasing guide should include instructions on updating trunk NEWS file
> https://bugs.launchpad.net/bugs/398653
> #473099 Incorrect markup in centralized workflow tutorial
> https://bugs.launchpad.net/bugs/473099
> #512385 Broken link to "quick start card" in online user guide
> https://bugs.launchpad.net/bugs/512385
>
>
> Mention Babune in the testing guide.
>

 merge: approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvqmQwACgkQJdeBCYSNAAMVOACeKZVFdoCXPXGdHD2n/C7yp2Dz
i3QAoKxsNHzIZTkopePafb+OMX9DPm/A
=xpco
-----END PGP SIGNATURE-----

review: Approve
Revision history for this message
Martin Pool (mbp) wrote :

submitted to PQM by hand.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'doc/developers/HACKING.txt'
2--- doc/developers/HACKING.txt 2010-04-16 12:59:03 +0000
3+++ doc/developers/HACKING.txt 2010-05-12 11:06:25 +0000
4@@ -179,11 +179,9 @@
5
6 * Bazaar - http://bazaar-vcs.org/
7
8-* Bundle Buggy - http://bundlebuggy.aaronbentley.com/
9-
10 * Patch Queue Manager - https://launchpad.net/pqm/
11
12-For further information, see http://bazaar-vcs.org/BzrDevelopment.
13+For further information, see <http://wiki.bazaar.canonical.com/BzrDevelopment>.
14
15
16
17@@ -272,239 +270,6 @@
18 <http://doc.bazaar-vcs.org/developers/overview.html>`_.
19
20
21-The Code Review Process
22-#######################
23-
24-All code changes coming in to Bazaar are reviewed by someone else.
25-Normally changes by core contributors are reviewed by one other core
26-developer, and changes from other people are reviewed by two core
27-developers. Use intelligent discretion if the patch is trivial.
28-
29-Good reviews do take time. They also regularly require a solid
30-understanding of the overall code base. In practice, this means a small
31-number of people often have a large review burden - with knowledge comes
32-responsibility. No one likes their merge requests sitting in a queue going
33-nowhere, so reviewing sooner rather than later is strongly encouraged.
34-
35-
36-
37-
38-
39-Review cover letters
40-====================
41-
42-Please put a "cover letter" on your merge request explaining:
43-
44-* the reason **why** you're making this change
45-
46-* **how** this change achieves this purpose
47-
48-* anything else you may have fixed in passing
49-
50-* anything significant that you thought of doing, such as a more
51- extensive fix or a different approach, but didn't or couldn't do now
52-
53-A good cover letter makes reviewers' lives easier because they can decide
54-from the letter whether they agree with the purpose and approach, and then
55-assess whether the patch actually does what the cover letter says.
56-Explaining any "drive-by fixes" or roads not taken may also avoid queries
57-from the reviewer. All in all this should give faster and better reviews.
58-Sometimes writing the cover letter helps the submitter realize something
59-else they need to do. The size of the cover letter should be proportional
60-to the size and complexity of the patch.
61-
62-
63-Reviewing proposed changes
64-==========================
65-
66-Anyone is welcome to review code, and reply to the thread with their
67-opinion or comments.
68-
69-The simplest way to review a proposed change is to just read the patch on
70-the list or in Bundle Buggy. For more complex changes it may be useful
71-to make a new working tree or branch from trunk, and merge the proposed
72-change into it, so you can experiment with the code or look at a wider
73-context.
74-
75-There are three main requirements for code to get in:
76-
77-* Doesn't reduce test coverage: if it adds new methods or commands,
78- there should be tests for them. There is a good test framework
79- and plenty of examples to crib from, but if you are having trouble
80- working out how to test something feel free to post a draft patch
81- and ask for help.
82-
83-* Doesn't reduce design clarity, such as by entangling objects
84- we're trying to separate. This is mostly something the more
85- experienced reviewers need to help check.
86-
87-* Improves bugs, features, speed, or code simplicity.
88-
89-Code that goes in should not degrade any of these aspects. Patches are
90-welcome that only cleanup the code without changing the external
91-behaviour. The core developers take care to keep the code quality high
92-and understandable while recognising that perfect is sometimes the enemy
93-of good.
94-
95-It is easy for reviews to make people notice other things which should be
96-fixed but those things should not hold up the original fix being accepted.
97-New things can easily be recorded in the Bug Tracker instead.
98-
99-It's normally much easier to review several smaller patches than one large
100-one. You might want to use ``bzr-loom`` to maintain threads of related
101-work, or submit a preparatory patch that will make your "real" change
102-easier.
103-
104-
105-Checklist for reviewers
106-=======================
107-
108-* Do you understand what the code's doing and why?
109-
110-* Will it perform reasonably for large inputs, both in memory size and
111- run time? Are there some scenarios where performance should be
112- measured?
113-
114-* Is it tested, and are the tests at the right level? Are there both
115- blackbox (command-line level) and API-oriented tests?
116-
117-* If this change will be visible to end users or API users, is it
118- appropriately documented in NEWS?
119-
120-* Does it meet the coding standards below?
121-
122-* If it changes the user-visible behaviour, does it update the help
123- strings and user documentation?
124-
125-* If it adds a new major concept or standard practice, does it update the
126- developer documentation?
127-
128-* (your ideas here...)
129-
130-
131-Reviews on Launchpad
132-====================
133-
134-From May 2009 on, we prefer people to propose code reviews through
135-Launchpad.
136-
137- * <https://launchpad.net/+tour/code-review>
138-
139- * <https://help.launchpad.net/Code/Review>
140-
141-Anyone can propose or comment on a merge proposal just by creating a
142-Launchpad account.
143-
144-There are two ways to create a new merge proposal: through the web
145-interface or by email.
146-
147-
148-Proposing a merge through the web
149----------------------------------
150-
151-To create the proposal through the web, first push your branch to Launchpad.
152-For example, a branch dealing with documentation belonging to the Launchpad
153-User mbp could be pushed as ::
154-
155- bzr push lp:~mbp/bzr/doc
156-
157-Then go to the branch's web page, which in this case would be
158-<https://code.launchpad.net/~mbp/bzr/doc>. You can simplify this step by just
159-running ::
160-
161- bzr lp-open
162-
163-You can then click "Propose for merging into another branch", and enter your
164-cover letter (see above) into the web form. Typically you'll want to merge
165-into ``~bzr/bzr/trunk`` which will be the default; you might also want to
166-nominate merging into a release branch for a bug fix. There is the option to
167-specify a specific reviewer or type of review, and you shouldn't normally
168-change those.
169-
170-Submitting the form takes you to the new page about the merge proposal
171-containing the diff of the changes, comments by interested people, and
172-controls to comment or vote on the change.
173-
174-Proposing a merge by mail
175--------------------------
176-
177-To propose a merge by mail, send a bundle to ``merge@code.launchpad.net``.
178-
179-You can generate a merge request like this::
180-
181- bzr send -o bug-1234.diff
182-
183-``bzr send`` can also send mail directly if you prefer; see the help.
184-
185-Reviewing changes
186------------------
187-
188-From <https://code.launchpad.net/bzr/+activereviews> you can see all
189-currently active reviews, and choose one to comment on. This page also
190-shows proposals that are now approved and should be merged by someone with
191-PQM access.
192-
193-
194-Reviews through Bundle Buggy
195-============================
196-
197-The Bundle Buggy tool used up to May 2009 is still available as a review
198-mechanism.
199-
200-Sending patches for review
201---------------------------
202-
203-If you'd like to propose a change, please post to the
204-bazaar@lists.canonical.com list with a bundle, patch, or link to a
205-branch. Put ``[PATCH]`` or ``[MERGE]`` in the subject so Bundle Buggy
206-can pick it out, and explain the change in the email message text.
207-Remember to update the NEWS file as part of your change if it makes any
208-changes visible to users or plugin developers. Please include a diff
209-against mainline if you're giving a link to a branch.
210-
211-You can generate a merge request like this::
212-
213- bzr send -o bug-1234.patch
214-
215-A ``.patch`` extension is recommended instead of .bundle as many mail clients
216-will send the latter as a binary file.
217-
218-``bzr send`` can also send mail directly if you prefer; see the help.
219-
220-Please do **NOT** put [PATCH] or [MERGE] in the subject line if you don't
221-want it to be merged. If you want comments from developers rather than
222-to be merged, you can put ``[RFC]`` in the subject line.
223-
224-If this change addresses a bug, please put the bug number in the subject
225-line too, in the form ``[#1]`` so that Bundle Buggy can recognize it.
226-
227-If the change is intended for a particular release mark that in the
228-subject too, e.g. ``[1.6]``.
229-Anyone can "vote" on the mailing list by expressing an opinion. Core
230-developers can also vote using Bundle Buggy. Here are the voting codes and
231-their explanations.
232-
233-:approve: Reviewer wants this submission merged.
234-:tweak: Reviewer wants this submission merged with small changes. (No
235- re-review required.)
236-:abstain: Reviewer does not intend to vote on this patch.
237-:resubmit: Please make changes and resubmit for review.
238-:reject: Reviewer doesn't want this kind of change merged.
239-:comment: Not really a vote. Reviewer just wants to comment, for now.
240-
241-If a change gets two approvals from core reviewers, and no rejections,
242-then it's OK to come in. Any of the core developers can bring it into the
243-bzr.dev trunk and backport it to maintenance branches if required. The
244-Release Manager will merge the change into the branch for a pending
245-release, if any. As a guideline, core developers usually merge their own
246-changes and volunteer to merge other contributions if they were the second
247-reviewer to agree to a change.
248-
249-To track the progress of proposed changes, use Bundle Buggy. See
250-http://bundlebuggy.aaronbentley.com/help for a link to all the
251-outstanding merge requests together with an explanation of the columns.
252-Bundle Buggy will also mail you a link to track just your change.
253-
254 Coding Style Guidelines
255 #######################
256
257
258=== added file 'doc/developers/code-review.txt'
259--- doc/developers/code-review.txt 1970-01-01 00:00:00 +0000
260+++ doc/developers/code-review.txt 2010-05-12 11:06:25 +0000
261@@ -0,0 +1,170 @@
262+The Code Review Process
263+#######################
264+
265+All code changes coming in to Bazaar are reviewed by someone else.
266+Normally changes by core contributors are reviewed by one other core
267+developer, and changes from other people are reviewed by two core
268+developers. Use intelligent discretion if the patch is trivial.
269+
270+Good reviews do take time. They also regularly require a solid
271+understanding of the overall code base. In practice, this means a small
272+number of people often have a large review burden - with knowledge comes
273+responsibility. No one likes their merge requests sitting in a queue going
274+nowhere, so reviewing sooner rather than later is strongly encouraged.
275+
276+
277+
278+Review cover letters
279+====================
280+
281+Please put a "cover letter" on your merge request explaining:
282+
283+* the reason **why** you're making this change
284+
285+* **how** this change achieves this purpose
286+
287+* anything else you may have fixed in passing
288+
289+* anything significant that you thought of doing, such as a more
290+ extensive fix or a different approach, but didn't or couldn't do now
291+
292+A good cover letter makes reviewers' lives easier because they can decide
293+from the letter whether they agree with the purpose and approach, and then
294+assess whether the patch actually does what the cover letter says.
295+Explaining any "drive-by fixes" or roads not taken may also avoid queries
296+from the reviewer. All in all this should give faster and better reviews.
297+Sometimes writing the cover letter helps the submitter realize something
298+else they need to do. The size of the cover letter should be proportional
299+to the size and complexity of the patch.
300+
301+
302+Reviewing proposed changes
303+==========================
304+
305+Anyone is welcome to review code, and reply to the thread with their
306+opinion or comments.
307+
308+The simplest way to review a proposed change is to just read the patch on
309+the list or in Bundle Buggy. For more complex changes it may be useful
310+to make a new working tree or branch from trunk, and merge the proposed
311+change into it, so you can experiment with the code or look at a wider
312+context.
313+
314+There are three main requirements for code to get in:
315+
316+* Doesn't reduce test coverage: if it adds new methods or commands,
317+ there should be tests for them. There is a good test framework
318+ and plenty of examples to crib from, but if you are having trouble
319+ working out how to test something feel free to post a draft patch
320+ and ask for help.
321+
322+* Doesn't reduce design clarity, such as by entangling objects
323+ we're trying to separate. This is mostly something the more
324+ experienced reviewers need to help check.
325+
326+* Improves bugs, features, speed, or code simplicity.
327+
328+Code that goes in should not degrade any of these aspects. Patches are
329+welcome that only cleanup the code without changing the external
330+behaviour. The core developers take care to keep the code quality high
331+and understandable while recognising that perfect is sometimes the enemy
332+of good.
333+
334+It is easy for reviews to make people notice other things which should be
335+fixed but those things should not hold up the original fix being accepted.
336+New things can easily be recorded in the Bug Tracker instead.
337+
338+It's normally much easier to review several smaller patches than one large
339+one. You might want to use ``bzr-loom`` to maintain threads of related
340+work, or submit a preparatory patch that will make your "real" change
341+easier.
342+
343+
344+Checklist for reviewers
345+=======================
346+
347+* Do you understand what the code's doing and why?
348+
349+* Will it perform reasonably for large inputs, both in memory size and
350+ run time? Are there some scenarios where performance should be
351+ measured?
352+
353+* Is it tested, and are the tests at the right level? Are there both
354+ blackbox (command-line level) and API-oriented tests?
355+
356+* If this change will be visible to end users or API users, is it
357+ appropriately documented in NEWS?
358+
359+* Does it meet the coding standards below?
360+
361+* If it changes the user-visible behaviour, does it update the help
362+ strings and user documentation?
363+
364+* If it adds a new major concept or standard practice, does it update the
365+ developer documentation?
366+
367+* (your ideas here...)
368+
369+
370+Reviews on Launchpad
371+====================
372+
373+From May 2009 on, we prefer people to propose code reviews through
374+Launchpad.
375+
376+ * <https://launchpad.net/+tour/code-review>
377+
378+ * <https://help.launchpad.net/Code/Review>
379+
380+Anyone can propose or comment on a merge proposal just by creating a
381+Launchpad account.
382+
383+There are two ways to create a new merge proposal: through the web
384+interface or by email.
385+
386+
387+Proposing a merge through the web
388+---------------------------------
389+
390+To create the proposal through the web, first push your branch to Launchpad.
391+For example, a branch dealing with documentation belonging to the Launchpad
392+User mbp could be pushed as ::
393+
394+ bzr push lp:~mbp/bzr/doc
395+
396+Then go to the branch's web page, which in this case would be
397+<https://code.launchpad.net/~mbp/bzr/doc>. You can simplify this step by just
398+running ::
399+
400+ bzr lp-open
401+
402+You can then click "Propose for merging into another branch", and enter your
403+cover letter (see above) into the web form. Typically you'll want to merge
404+into ``~bzr/bzr/trunk`` which will be the default; you might also want to
405+nominate merging into a release branch for a bug fix. There is the option to
406+specify a specific reviewer or type of review, and you shouldn't normally
407+change those.
408+
409+Submitting the form takes you to the new page about the merge proposal
410+containing the diff of the changes, comments by interested people, and
411+controls to comment or vote on the change.
412+
413+Proposing a merge by mail
414+-------------------------
415+
416+To propose a merge by mail, send a bundle to ``merge@code.launchpad.net``.
417+
418+You can generate a merge request like this::
419+
420+ bzr send -o bug-1234.diff
421+
422+``bzr send`` can also send mail directly if you prefer; see the help.
423+
424+Reviewing changes
425+-----------------
426+
427+From <https://code.launchpad.net/bzr/+activereviews> you can see all
428+currently active reviews, and choose one to comment on. This page also
429+shows proposals that are now approved and should be merged by someone with
430+PQM access.
431+
432
433=== modified file 'doc/developers/index-plain.txt'
434--- doc/developers/index-plain.txt 2010-02-23 04:12:26 +0000
435+++ doc/developers/index-plain.txt 2010-05-12 11:06:25 +0000
436@@ -24,6 +24,8 @@
437
438 * `Testing <testing.html>`_ |--| Guide to writing tests for Bazaar.
439
440+* `Code Review <code-review.html>`_.
441+
442 * `Writing plugins <http://doc.bazaar.canonical.com/plugins/en/plugin-development.html>`_
443 |--| specific advice on writing Bazaar plugins. (web link)
444
445
446=== modified file 'doc/developers/index.txt'
447--- doc/developers/index.txt 2010-02-23 04:12:26 +0000
448+++ doc/developers/index.txt 2010-05-12 11:06:25 +0000
449@@ -23,6 +23,7 @@
450 bug-handling
451 HACKING
452 testing
453+ code-review
454
455 * `Contributing to Bazaar Documentation <http://wiki.bazaar.canonical.com/ContributingToTheDocs>`_ (wiki)
456
457
458=== modified file 'doc/developers/testing.txt'
459--- doc/developers/testing.txt 2010-05-06 23:41:35 +0000
460+++ doc/developers/testing.txt 2010-05-12 11:06:25 +0000
461@@ -6,7 +6,7 @@
462 The Importance of Testing
463 =========================
464
465-Reliability is a critical success factor for any Version Control System.
466+Reliability is a critical success factor for any version control system.
467 We want Bazaar to be highly reliable across multiple platforms while
468 evolving over time to meet the needs of its community.
469
470@@ -183,6 +183,16 @@
471
472 .. _testrepository: https://launchpad.net/testrepository
473
474+
475+Babune continuous integration
476+-----------------------------
477+
478+We have a Hudson continuous-integration system that automatically runs
479+tests across various platforms. In the future we plan to add more
480+combinations including testing plugins. See
481+<http://babune.ladeuil.net:24842/>. (Babune = Bazaar Buildbot Network.)
482+
483+
484 Writing Tests
485 =============
486