Bazaar

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

  1. doc
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Merged
Approved by: Martin Pool
Approved revision: no longer in the source branch.
Merged at revision: 5252
Proposed branch: lp:~mbp/bzr/doc
Merge into: lp:bzr
Diff against target: 1365 lines (+577/-569)
7 files modified
doc/developers/HACKING.txt (+27/-433)
doc/developers/code-review.txt (+31/-97)
doc/developers/code-style.txt (+413/-0)
doc/developers/index-plain.txt (+2/-0)
doc/developers/index.txt (+1/-0)
doc/developers/integration.txt (+38/-10)
doc/developers/overview.txt (+65/-29)
To merge this branch: bzr merge lp:~mbp/bzr/doc
Reviewer Review Type Date Requested Status
Robert Collins (community) Needs Fixing
bzr-core Pending
Review via email: mp+25400@code.launchpad.net

This proposal supersedes a proposal from 2010-05-14.

Commit message

better docs on contributing, code reviews, and using bzrlib

Description of the change

Better docs on contributing, code reviews, and using bzrlib.

Includes some class descriptions moved from very old versions on the wiki.

This is moving towards having the following distinct but related developer docs:

 * code style guidelines
 * how to contribute a change to bzr
 * how to review proposed changes
 * recipes for using bzrlib from plugins
 * an overview of the whole library, for people either working on it or using it

Obviously one can slice it various ways but I think this gets it better aligned with audience needs.

Also:
 * support python2.4-2.6 (and eventually 3.0) - any other additions there?
 * remove some obsolete content about bundle buggy (bless its socks)

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote : Posted in a previous version of this proposal

Sure, looks nice. +1

Revision history for this message
Robert Collins (lifeless) wrote :

This: + * Allows write locks to be taken out to prevent concurrent alterations to the repository.
is stale / misleading - we're granular now.

Could you please add something about setting the commit message in the merge proposal?

Other than that, a cursory overview seems fine.

Revision history for this message
Robert Collins (lifeless) :
review: Needs Fixing
Revision history for this message
Robert Collins (lifeless) wrote :

I voted needs fixing a while back, which is 'tweak' in BB parlance, so you should be fine to land this. I can land it if you'd like.

Revision history for this message
Robert Collins (lifeless) wrote :

Oh, except you don't seem to have addressed "Could you please add something about setting the commit message in the merge proposal?" that I can see. I feel this is important with our increased use of hydrazine.

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

> Oh, except you don't seem to have addressed "Could you please add something
> about setting the commit message in the merge proposal?" that I can see. I
> feel this is important with our increased use of hydrazine.

I agree that would be useful; I'll send this up first and try that in a later iteration about contributions.

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

sent to pqm by email

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-05-12 11:04:21 +0000
3+++ doc/developers/HACKING.txt 2010-05-23 20:46:24 +0000
4@@ -35,8 +35,6 @@
5
6 * Bug Tracker for the core product - https://bugs.launchpad.net/bzr/
7
8-* Blueprint Tracker for the core product - https://blueprints.launchpad.net/bzr/
9-
10 If nothing else, perhaps you'll find inspiration in how other developers
11 have solved their challenges.
12
13@@ -128,6 +126,9 @@
14 "Propose for merging into another branch". Select "~bzr/bzr/trunk" to hand
15 your changes off to the Bazaar developers for review and merging.
16
17+Alternatively, after pushing you can use the ``lp-propose`` command to
18+create the merge proposal.
19+
20 Using a meaningful name for your branch will help you and the reviewer(s)
21 better track the submission. Use a very succint description of your submission
22 and prefix it with bug number if needed (lp:~mbp/bzr/484558-merge-directory
23@@ -135,6 +136,30 @@
24 (lp:~jameinel/bzr/export-file-511987).
25
26
27+Review cover letters
28+--------------------
29+
30+Please put a "cover letter" on your merge request explaining:
31+
32+* the reason **why** you're making this change
33+
34+* **how** this change achieves this purpose
35+
36+* anything else you may have fixed in passing
37+
38+* anything significant that you thought of doing, such as a more
39+ extensive fix or a different approach, but didn't or couldn't do now
40+
41+A good cover letter makes reviewers' lives easier because they can decide
42+from the letter whether they agree with the purpose and approach, and then
43+assess whether the patch actually does what the cover letter says.
44+Explaining any "drive-by fixes" or roads not taken may also avoid queries
45+from the reviewer. All in all this should give faster and better reviews.
46+Sometimes writing the cover letter helps the submitter realize something
47+else they need to do. The size of the cover letter should be proportional
48+to the size and complexity of the patch.
49+
50+
51 Why make a local copy of bzr.dev?
52 ---------------------------------
53
54@@ -270,363 +295,6 @@
55 <http://doc.bazaar-vcs.org/developers/overview.html>`_.
56
57
58-Coding Style Guidelines
59-#######################
60-
61-hasattr and getattr
62-===================
63-
64-``hasattr`` should not be used because it swallows exceptions including
65-``KeyboardInterrupt``. Instead, say something like ::
66-
67- if getattr(thing, 'name', None) is None
68-
69-
70-Code layout
71-===========
72-
73-Please write PEP-8__ compliant code.
74-
75-__ http://www.python.org/peps/pep-0008.html
76-
77-One often-missed requirement is that the first line of docstrings
78-should be a self-contained one-sentence summary.
79-
80-We use 4 space indents for blocks, and never use tab characters. (In vim,
81-``set expandtab``.)
82-
83-Trailing white space should be avoided, but is allowed.
84-You should however not make lots of unrelated white space changes.
85-
86-Unix style newlines (LF) are used.
87-
88-Each file must have a newline at the end of it.
89-
90-Lines should be no more than 79 characters if at all possible.
91-Lines that continue a long statement may be indented in either of
92-two ways:
93-
94-within the parenthesis or other character that opens the block, e.g.::
95-
96- my_long_method(arg1,
97- arg2,
98- arg3)
99-
100-or indented by four spaces::
101-
102- my_long_method(arg1,
103- arg2,
104- arg3)
105-
106-The first is considered clearer by some people; however it can be a bit
107-harder to maintain (e.g. when the method name changes), and it does not
108-work well if the relevant parenthesis is already far to the right. Avoid
109-this::
110-
111- self.legbone.kneebone.shinbone.toebone.shake_it(one,
112- two,
113- three)
114-
115-but rather ::
116-
117- self.legbone.kneebone.shinbone.toebone.shake_it(one,
118- two,
119- three)
120-
121-or ::
122-
123- self.legbone.kneebone.shinbone.toebone.shake_it(
124- one, two, three)
125-
126-For long lists, we like to add a trailing comma and put the closing
127-character on the following line. This makes it easier to add new items in
128-future::
129-
130- from bzrlib.goo import (
131- jam,
132- jelly,
133- marmalade,
134- )
135-
136-There should be spaces between function parameters, but not between the
137-keyword name and the value::
138-
139- call(1, 3, cheese=quark)
140-
141-In emacs::
142-
143- ;(defface my-invalid-face
144- ; '((t (:background "Red" :underline t)))
145- ; "Face used to highlight invalid constructs or other uglyties"
146- ; )
147-
148- (defun my-python-mode-hook ()
149- ;; setup preferred indentation style.
150- (setq fill-column 79)
151- (setq indent-tabs-mode nil) ; no tabs, never, I will not repeat
152- ; (font-lock-add-keywords 'python-mode
153- ; '(("^\\s *\t" . 'my-invalid-face) ; Leading tabs
154- ; ("[ \t]+$" . 'my-invalid-face) ; Trailing spaces
155- ; ("^[ \t]+$" . 'my-invalid-face)); Spaces only
156- ; )
157- )
158-
159- (add-hook 'python-mode-hook 'my-python-mode-hook)
160-
161-The lines beginning with ';' are comments. They can be activated
162-if one want to have a strong notice of some tab/space usage
163-violations.
164-
165-
166-Module Imports
167-==============
168-
169-* Imports should be done at the top-level of the file, unless there is
170- a strong reason to have them lazily loaded when a particular
171- function runs. Import statements have a cost, so try to make sure
172- they don't run inside hot functions.
173-
174-* Module names should always be given fully-qualified,
175- i.e. ``bzrlib.hashcache`` not just ``hashcache``.
176-
177-
178-Naming
179-======
180-
181-Functions, methods or members that are relatively private are given
182-a leading underscore prefix. Names without a leading underscore are
183-public not just across modules but to programmers using bzrlib as an
184-API.
185-
186-We prefer class names to be concatenated capital words (``TestCase``)
187-and variables, methods and functions to be lowercase words joined by
188-underscores (``revision_id``, ``get_revision``).
189-
190-For the purposes of naming some names are treated as single compound
191-words: "filename", "revno".
192-
193-Consider naming classes as nouns and functions/methods as verbs.
194-
195-Try to avoid using abbreviations in names, because there can be
196-inconsistency if other people use the full name.
197-
198-
199-Standard Names
200-==============
201-
202-``revision_id`` not ``rev_id`` or ``revid``
203-
204-Functions that transform one thing to another should be named ``x_to_y``
205-(not ``x2y`` as occurs in some old code.)
206-
207-
208-Destructors
209-===========
210-
211-Python destructors (``__del__``) work differently to those of other
212-languages. In particular, bear in mind that destructors may be called
213-immediately when the object apparently becomes unreferenced, or at some
214-later time, or possibly never at all. Therefore we have restrictions on
215-what can be done inside them.
216-
217- 0. If you think you need to use a ``__del__`` method ask another
218- developer for alternatives. If you do need to use one, explain
219- why in a comment.
220-
221- 1. Never rely on a ``__del__`` method running. If there is code that
222- must run, do it from a ``finally`` block instead.
223-
224- 2. Never ``import`` from inside a ``__del__`` method, or you may crash the
225- interpreter!!
226-
227- 3. In some places we raise a warning from the destructor if the object
228- has not been cleaned up or closed. This is considered OK: the warning
229- may not catch every case but it's still useful sometimes.
230-
231-
232-Cleanup methods
233-===============
234-
235-Often when something has failed later code, including cleanups invoked
236-from ``finally`` blocks, will fail too. These secondary failures are
237-generally uninteresting compared to the original exception. So use the
238-``only_raises`` decorator (from ``bzrlib.decorators``) for methods that
239-are typically called in ``finally`` blocks, such as ``unlock`` methods.
240-For example, ``@only_raises(LockNotHeld, LockBroken)``. All errors that
241-are unlikely to be a knock-on failure from a previous failure should be
242-allowed.
243-
244-
245-Factories
246-=========
247-
248-In some places we have variables which point to callables that construct
249-new instances. That is to say, they can be used a lot like class objects,
250-but they shouldn't be *named* like classes::
251-
252-> I think that things named FooBar should create instances of FooBar when
253-> called. Its plain confusing for them to do otherwise. When we have
254-> something that is going to be used as a class - that is, checked for via
255-> isinstance or other such idioms, them I would call it foo_class, so that
256-> it is clear that a callable is not sufficient. If it is only used as a
257-> factory, then yes, foo_factory is what I would use.
258-
259-
260-Registries
261-==========
262-
263-Several places in Bazaar use (or will use) a registry, which is a
264-mapping from names to objects or classes. The registry allows for
265-loading in registered code only when it's needed, and keeping
266-associated information such as a help string or description.
267-
268-
269-InterObject and multiple dispatch
270-=================================
271-
272-The ``InterObject`` provides for two-way `multiple dispatch`__: matching
273-up for example a source and destination repository to find the right way
274-to transfer data between them.
275-
276-.. __: http://en.wikipedia.org/wiki/Multiple_dispatch
277-
278-There is a subclass ``InterObject`` classes for each type of object that is
279-dispatched this way, e.g. ``InterRepository``. Calling ``.get()`` on this
280-class will return an ``InterObject`` instance providing the best match for
281-those parameters, and this instance then has methods for operations
282-between the objects.
283-
284-::
285-
286- inter = InterRepository.get(source_repo, target_repo)
287- inter.fetch(revision_id)
288-
289-``InterRepository`` also acts as a registry-like object for its
290-subclasses, and they can be added through ``.register_optimizer``. The
291-right one to run is selected by asking each class, in reverse order of
292-registration, whether it ``.is_compatible`` with the relevant objects.
293-
294-Lazy Imports
295-============
296-
297-To make startup time faster, we use the ``bzrlib.lazy_import`` module to
298-delay importing modules until they are actually used. ``lazy_import`` uses
299-the same syntax as regular python imports. So to import a few modules in a
300-lazy fashion do::
301-
302- from bzrlib.lazy_import import lazy_import
303- lazy_import(globals(), """
304- import os
305- import subprocess
306- import sys
307- import time
308-
309- from bzrlib import (
310- errors,
311- transport,
312- revision as _mod_revision,
313- )
314- import bzrlib.transport
315- import bzrlib.xml5
316- """)
317-
318-At this point, all of these exist as a ``ImportReplacer`` object, ready to
319-be imported once a member is accessed. Also, when importing a module into
320-the local namespace, which is likely to clash with variable names, it is
321-recommended to prefix it as ``_mod_<module>``. This makes it clearer that
322-the variable is a module, and these object should be hidden anyway, since
323-they shouldn't be imported into other namespaces.
324-
325-While it is possible for ``lazy_import()`` to import members of a module
326-when using the ``from module import member`` syntax, it is recommended to
327-only use that syntax to load sub modules ``from module import submodule``.
328-This is because variables and classes can frequently be used without
329-needing a sub-member for example::
330-
331- lazy_import(globals(), """
332- from module import MyClass
333- """)
334-
335- def test(x):
336- return isinstance(x, MyClass)
337-
338-This will incorrectly fail, because ``MyClass`` is a ``ImportReplacer``
339-object, rather than the real class.
340-
341-It also is incorrect to assign ``ImportReplacer`` objects to other variables.
342-Because the replacer only knows about the original name, it is unable to
343-replace other variables. The ``ImportReplacer`` class will raise an
344-``IllegalUseOfScopeReplacer`` exception if it can figure out that this
345-happened. But it requires accessing a member more than once from the new
346-variable, so some bugs are not detected right away.
347-
348-
349-The Null revision
350-=================
351-
352-The null revision is the ancestor of all revisions. Its revno is 0, its
353-revision-id is ``null:``, and its tree is the empty tree. When referring
354-to the null revision, please use ``bzrlib.revision.NULL_REVISION``. Old
355-code sometimes uses ``None`` for the null revision, but this practice is
356-being phased out.
357-
358-
359-Object string representations
360-=============================
361-
362-Python prints objects using their ``__repr__`` method when they are
363-written to logs, exception tracebacks, or the debugger. We want
364-objects to have useful representations to help in determining what went
365-wrong.
366-
367-If you add a new class you should generally add a ``__repr__`` method
368-unless there is an adequate method in a parent class. There should be a
369-test for the repr.
370-
371-Representations should typically look like Python constructor syntax, but
372-they don't need to include every value in the object and they don't need
373-to be able to actually execute. They're to be read by humans, not
374-machines. Don't hardcode the classname in the format, so that we get the
375-correct value if the method is inherited by a subclass. If you're
376-printing attributes of the object, including strings, you should normally
377-use ``%r`` syntax (to call their repr in turn).
378-
379-Try to avoid the representation becoming more than one or two lines long.
380-(But balance this against including useful information, and simplicity of
381-implementation.)
382-
383-Because repr methods are often called when something has already gone
384-wrong, they should be written somewhat more defensively than most code.
385-The object may be half-initialized or in some other way in an illegal
386-state. The repr method shouldn't raise an exception, or it may hide the
387-(probably more useful) underlying exception.
388-
389-Example::
390-
391- def __repr__(self):
392- return '%s(%r)' % (self.__class__.__name__,
393- self._transport)
394-
395-
396-Exception handling
397-==================
398-
399-A bare ``except`` statement will catch all exceptions, including ones that
400-really should terminate the program such as ``MemoryError`` and
401-``KeyboardInterrupt``. They should rarely be used unless the exception is
402-later re-raised. Even then, think about whether catching just
403-``Exception`` (which excludes system errors in Python2.5 and later) would
404-be better.
405-
406-
407-Test coverage
408-=============
409-
410-All code should be exercised by the test suite. See the `Bazaar Testing
411-Guide <http://doc.bazaar-vcs.org/developers/testing.html>`_ for detailed
412-information about writing tests.
413-
414-
415 Core Topics
416 ###########
417
418@@ -891,35 +559,6 @@
419 final fullstop. If long, they may contain newlines to break the text.
420
421
422-Assertions
423-==========
424-
425-Do not use the Python ``assert`` statement, either in tests or elsewhere.
426-A source test checks that it is not used. It is ok to explicitly raise
427-AssertionError.
428-
429-Rationale:
430-
431- * It makes the behaviour vary depending on whether bzr is run with -O
432- or not, therefore giving a chance for bugs that occur in one case or
433- the other, several of which have already occurred: assertions with
434- side effects, code which can't continue unless the assertion passes,
435- cases where we should give the user a proper message rather than an
436- assertion failure.
437- * It's not that much shorter than an explicit if/raise.
438- * It tends to lead to fuzzy thinking about whether the check is
439- actually needed or not, and whether it's an internal error or not
440- * It tends to cause look-before-you-leap patterns.
441- * It's unsafe if the check is needed to protect the integrity of the
442- user's data.
443- * It tends to give poor messages since the developer can get by with
444- no explanatory text at all.
445- * We can't rely on people always running with -O in normal use, so we
446- can't use it for tests that are actually expensive.
447- * Expensive checks that help developers are better turned on from the
448- test suite or a -D flag.
449- * If used instead of ``self.assert*()`` in tests it makes them falsely pass with -O.
450-
451
452 Documenting Changes
453 ===================
454@@ -1143,17 +782,6 @@
455 valid characters are generated where possible.
456
457
458-Portability Tips
459-================
460-
461-The ``bzrlib.osutils`` module has many useful helper functions, including
462-some more portable variants of functions in the standard library.
463-
464-In particular, don't use ``shutil.rmtree`` unless it's acceptable for it
465-to fail on Windows if some files are readonly or still open elsewhere.
466-Use ``bzrlib.osutils.rmtree`` instead.
467-
468-
469 C Extension Modules
470 ===================
471
472@@ -1411,44 +1039,10 @@
473 results.
474
475
476-Reviewing Blueprints
477-====================
478-
479-Blueprint Tracking Using Launchpad
480-----------------------------------
481-
482-New features typically require a fair amount of discussion, design and
483-debate. For Bazaar, that information is often captured in a so-called
484-"blueprint" on our Wiki. Overall tracking of blueprints and their status
485-is done using Launchpad's relevant tracker,
486-https://blueprints.launchpad.net/bzr/. Once a blueprint for ready for
487-review, please announce it on the mailing list.
488-
489-Alternatively, send an email beginning with [RFC] with the proposal to the
490-list. In some cases, you may wish to attach proposed code or a proposed
491-developer document if that best communicates the idea. Debate can then
492-proceed using the normal merge review processes.
493-
494-
495-Recording Blueprint Review Feedback
496------------------------------------
497-
498-Unlike its Bug Tracker, Launchpad's Blueprint Tracker doesn't currently
499-(Jun 2007) support a chronological list of comment responses. Review
500-feedback can either be recorded on the Wiki hosting the blueprints or by
501-using Launchpad's whiteboard feature.
502-
503-
504 Planning Releases
505 =================
506
507
508-Using Releases and Milestones in Launchpad
509-------------------------------------------
510-
511-TODO ... (Exact policies still under discussion)
512-
513-
514 Bug Triage
515 ----------
516
517
518=== modified file 'doc/developers/code-review.txt'
519--- doc/developers/code-review.txt 2010-05-12 11:04:21 +0000
520+++ doc/developers/code-review.txt 2010-05-23 20:46:24 +0000
521@@ -1,55 +1,28 @@
522-The Code Review Process
523-#######################
524-
525-All code changes coming in to Bazaar are reviewed by someone else.
526+Reviewing proposed changes to Bazaar
527+####################################
528+
529+All non-trivial code changes coming in to Bazaar are reviewed by someone else.
530+
531+Anyone is welcome to review any patch. You don't need to have a full
532+understanding of the codebase to find problems in the code, the documentation,
533+or the concept of the patch.
534+
535 Normally changes by core contributors are reviewed by one other core
536 developer, and changes from other people are reviewed by two core
537-developers. Use intelligent discretion if the patch is trivial.
538-
539-Good reviews do take time. They also regularly require a solid
540-understanding of the overall code base. In practice, this means a small
541-number of people often have a large review burden - with knowledge comes
542-responsibility. No one likes their merge requests sitting in a queue going
543-nowhere, so reviewing sooner rather than later is strongly encouraged.
544-
545-
546-
547-Review cover letters
548-====================
549-
550-Please put a "cover letter" on your merge request explaining:
551-
552-* the reason **why** you're making this change
553-
554-* **how** this change achieves this purpose
555-
556-* anything else you may have fixed in passing
557-
558-* anything significant that you thought of doing, such as a more
559- extensive fix or a different approach, but didn't or couldn't do now
560-
561-A good cover letter makes reviewers' lives easier because they can decide
562-from the letter whether they agree with the purpose and approach, and then
563-assess whether the patch actually does what the cover letter says.
564-Explaining any "drive-by fixes" or roads not taken may also avoid queries
565-from the reviewer. All in all this should give faster and better reviews.
566-Sometimes writing the cover letter helps the submitter realize something
567-else they need to do. The size of the cover letter should be proportional
568-to the size and complexity of the patch.
569+developers. Use intelligent discretion about whether if the patch is trivial.
570+
571+No one likes their merge requests sitting in a queue going nowhere: this
572+is pure waste. We prioritize reviewing existing proposals.
573+Canonical dedicates some staff time to providing prompt helpful reviews.
574+(See <http://wiki.bazaar.canonical.com/PatchPilot/>.)
575+
576+From late 2009 on, we do all our code reviews through Launchpad's
577+merge proposal interface.
578
579
580 Reviewing proposed changes
581 ==========================
582
583-Anyone is welcome to review code, and reply to the thread with their
584-opinion or comments.
585-
586-The simplest way to review a proposed change is to just read the patch on
587-the list or in Bundle Buggy. For more complex changes it may be useful
588-to make a new working tree or branch from trunk, and merge the proposed
589-change into it, so you can experiment with the code or look at a wider
590-context.
591-
592 There are three main requirements for code to get in:
593
594 * Doesn't reduce test coverage: if it adds new methods or commands,
595@@ -72,7 +45,7 @@
596
597 It is easy for reviews to make people notice other things which should be
598 fixed but those things should not hold up the original fix being accepted.
599-New things can easily be recorded in the Bug Tracker instead.
600+New things can easily be recorded in the bug tracker instead.
601
602 It's normally much easier to review several smaller patches than one large
603 one. You might want to use ``bzr-loom`` to maintain threads of related
604@@ -109,62 +82,23 @@
605 Reviews on Launchpad
606 ====================
607
608-From May 2009 on, we prefer people to propose code reviews through
609-Launchpad.
610-
611- * <https://launchpad.net/+tour/code-review>
612-
613- * <https://help.launchpad.net/Code/Review>
614-
615 Anyone can propose or comment on a merge proposal just by creating a
616 Launchpad account.
617
618-There are two ways to create a new merge proposal: through the web
619-interface or by email.
620-
621-
622-Proposing a merge through the web
623----------------------------------
624-
625-To create the proposal through the web, first push your branch to Launchpad.
626-For example, a branch dealing with documentation belonging to the Launchpad
627-User mbp could be pushed as ::
628-
629- bzr push lp:~mbp/bzr/doc
630-
631-Then go to the branch's web page, which in this case would be
632-<https://code.launchpad.net/~mbp/bzr/doc>. You can simplify this step by just
633-running ::
634-
635- bzr lp-open
636-
637-You can then click "Propose for merging into another branch", and enter your
638-cover letter (see above) into the web form. Typically you'll want to merge
639-into ``~bzr/bzr/trunk`` which will be the default; you might also want to
640-nominate merging into a release branch for a bug fix. There is the option to
641-specify a specific reviewer or type of review, and you shouldn't normally
642-change those.
643-
644-Submitting the form takes you to the new page about the merge proposal
645-containing the diff of the changes, comments by interested people, and
646-controls to comment or vote on the change.
647-
648-Proposing a merge by mail
649--------------------------
650-
651-To propose a merge by mail, send a bundle to ``merge@code.launchpad.net``.
652-
653-You can generate a merge request like this::
654-
655- bzr send -o bug-1234.diff
656-
657-``bzr send`` can also send mail directly if you prefer; see the help.
658-
659-Reviewing changes
660------------------
661-
662 From <https://code.launchpad.net/bzr/+activereviews> you can see all
663 currently active reviews, and choose one to comment on. This page also
664 shows proposals that are now approved and should be merged by someone with
665 PQM access.
666
667+<https://help.launchpad.net/Code/Review> explains the various merge proposal
668+states. Note that we don't use state *Approved* until the patch is completely
669+ready to merge.
670+
671+
672+Landing approved changes
673+========================
674+
675+Once a merge proposal is approved and finished, it's sent to PQM (the patch
676+queue manager) which will automatically test and integrate it. The recommended
677+way to start this off is by running the ``feed-pqm`` script from
678+<https://launchpad.net/hydrazine/>.
679
680=== added file 'doc/developers/code-style.txt'
681--- doc/developers/code-style.txt 1970-01-01 00:00:00 +0000
682+++ doc/developers/code-style.txt 2010-05-23 20:46:24 +0000
683@@ -0,0 +1,413 @@
684+***********************
685+Bazaar Code Style Guide
686+***********************
687+
688+Code layout
689+===========
690+
691+Please write PEP-8__ compliant code.
692+
693+__ http://www.python.org/peps/pep-0008.html
694+
695+One often-missed requirement is that the first line of docstrings
696+should be a self-contained one-sentence summary.
697+
698+We use 4 space indents for blocks, and never use tab characters. (In vim,
699+``set expandtab``.)
700+
701+Trailing white space should be avoided, but is allowed.
702+You should however not make lots of unrelated white space changes.
703+
704+Unix style newlines (LF) are used.
705+
706+Each file must have a newline at the end of it.
707+
708+Lines should be no more than 79 characters if at all possible.
709+Lines that continue a long statement may be indented in either of
710+two ways:
711+
712+within the parenthesis or other character that opens the block, e.g.::
713+
714+ my_long_method(arg1,
715+ arg2,
716+ arg3)
717+
718+or indented by four spaces::
719+
720+ my_long_method(arg1,
721+ arg2,
722+ arg3)
723+
724+The first is considered clearer by some people; however it can be a bit
725+harder to maintain (e.g. when the method name changes), and it does not
726+work well if the relevant parenthesis is already far to the right. Avoid
727+this::
728+
729+ self.legbone.kneebone.shinbone.toebone.shake_it(one,
730+ two,
731+ three)
732+
733+but rather ::
734+
735+ self.legbone.kneebone.shinbone.toebone.shake_it(one,
736+ two,
737+ three)
738+
739+or ::
740+
741+ self.legbone.kneebone.shinbone.toebone.shake_it(
742+ one, two, three)
743+
744+For long lists, we like to add a trailing comma and put the closing
745+character on the following line. This makes it easier to add new items in
746+future::
747+
748+ from bzrlib.goo import (
749+ jam,
750+ jelly,
751+ marmalade,
752+ )
753+
754+There should be spaces between function parameters, but not between the
755+keyword name and the value::
756+
757+ call(1, 3, cheese=quark)
758+
759+
760+Python versions
761+===============
762+
763+Bazaar supports Python from 2.4 through 2.6, and in the future we want to
764+support Python 3.0. Avoid using language features added in 2.5 or 2.6, or
765+features deprecated in Python 3.0. (You can check v3 compatibility using
766+the ``-3`` option of Python2.6.)
767+
768+Specifically:
769+
770+ * don't use the ``with`` statement
771+ * don't ``from . import``
772+
773+
774+hasattr and getattr
775+===================
776+
777+``hasattr`` should not be used because it swallows exceptions including
778+``KeyboardInterrupt``. Instead, say something like ::
779+
780+ if getattr(thing, 'name', None) is None
781+
782+
783+Module Imports
784+==============
785+
786+* Imports should be done at the top-level of the file, unless there is
787+ a strong reason to have them lazily loaded when a particular
788+ function runs. Import statements have a cost, so try to make sure
789+ they don't run inside hot functions.
790+
791+* Module names should always be given fully-qualified,
792+ i.e. ``bzrlib.hashcache`` not just ``hashcache``.
793+
794+
795+Naming
796+======
797+
798+Functions, methods or members that are relatively private are given
799+a leading underscore prefix. Names without a leading underscore are
800+public not just across modules but to programmers using bzrlib as an
801+API.
802+
803+We prefer class names to be concatenated capital words (``TestCase``)
804+and variables, methods and functions to be lowercase words joined by
805+underscores (``revision_id``, ``get_revision``).
806+
807+For the purposes of naming some names are treated as single compound
808+words: "filename", "revno".
809+
810+Consider naming classes as nouns and functions/methods as verbs.
811+
812+Try to avoid using abbreviations in names, because there can be
813+inconsistency if other people use the full name.
814+
815+
816+Standard Names
817+==============
818+
819+``revision_id`` not ``rev_id`` or ``revid``
820+
821+Functions that transform one thing to another should be named ``x_to_y``
822+(not ``x2y`` as occurs in some old code.)
823+
824+
825+Destructors
826+===========
827+
828+Python destructors (``__del__``) work differently to those of other
829+languages. In particular, bear in mind that destructors may be called
830+immediately when the object apparently becomes unreferenced, or at some
831+later time, or possibly never at all. Therefore we have restrictions on
832+what can be done inside them.
833+
834+ 0. If you think you need to use a ``__del__`` method ask another
835+ developer for alternatives. If you do need to use one, explain
836+ why in a comment.
837+
838+ 1. Never rely on a ``__del__`` method running. If there is code that
839+ must run, do it from a ``finally`` block instead.
840+
841+ 2. Never ``import`` from inside a ``__del__`` method, or you may crash the
842+ interpreter!!
843+
844+ 3. In some places we raise a warning from the destructor if the object
845+ has not been cleaned up or closed. This is considered OK: the warning
846+ may not catch every case but it's still useful sometimes.
847+
848+
849+Cleanup methods
850+===============
851+
852+Often when something has failed later code, including cleanups invoked
853+from ``finally`` blocks, will fail too. These secondary failures are
854+generally uninteresting compared to the original exception. So use the
855+``only_raises`` decorator (from ``bzrlib.decorators``) for methods that
856+are typically called in ``finally`` blocks, such as ``unlock`` methods.
857+For example, ``@only_raises(LockNotHeld, LockBroken)``. All errors that
858+are unlikely to be a knock-on failure from a previous failure should be
859+allowed.
860+
861+
862+Factories
863+=========
864+
865+In some places we have variables which point to callables that construct
866+new instances. That is to say, they can be used a lot like class objects,
867+but they shouldn't be *named* like classes. Things called ``FooBar`` should
868+create an instance of ``FooBar``. A factory method that might create a
869+``FooBar`` or might make something else should be called ``foo_factory``.
870+
871+
872+Registries
873+==========
874+
875+Several places in Bazaar use (or will use) a registry, which is a
876+mapping from names to objects or classes. The registry allows for
877+loading in registered code only when it's needed, and keeping
878+associated information such as a help string or description.
879+
880+
881+InterObject and multiple dispatch
882+=================================
883+
884+The ``InterObject`` provides for two-way `multiple dispatch`__: matching
885+up for example a source and destination repository to find the right way
886+to transfer data between them.
887+
888+.. __: http://en.wikipedia.org/wiki/Multiple_dispatch
889+
890+There is a subclass ``InterObject`` classes for each type of object that is
891+dispatched this way, e.g. ``InterRepository``. Calling ``.get()`` on this
892+class will return an ``InterObject`` instance providing the best match for
893+those parameters, and this instance then has methods for operations
894+between the objects.
895+
896+::
897+
898+ inter = InterRepository.get(source_repo, target_repo)
899+ inter.fetch(revision_id)
900+
901+``InterRepository`` also acts as a registry-like object for its
902+subclasses, and they can be added through ``.register_optimizer``. The
903+right one to run is selected by asking each class, in reverse order of
904+registration, whether it ``.is_compatible`` with the relevant objects.
905+
906+Lazy Imports
907+============
908+
909+To make startup time faster, we use the ``bzrlib.lazy_import`` module to
910+delay importing modules until they are actually used. ``lazy_import`` uses
911+the same syntax as regular python imports. So to import a few modules in a
912+lazy fashion do::
913+
914+ from bzrlib.lazy_import import lazy_import
915+ lazy_import(globals(), """
916+ import os
917+ import subprocess
918+ import sys
919+ import time
920+
921+ from bzrlib import (
922+ errors,
923+ transport,
924+ revision as _mod_revision,
925+ )
926+ import bzrlib.transport
927+ import bzrlib.xml5
928+ """)
929+
930+At this point, all of these exist as a ``ImportReplacer`` object, ready to
931+be imported once a member is accessed. Also, when importing a module into
932+the local namespace, which is likely to clash with variable names, it is
933+recommended to prefix it as ``_mod_<module>``. This makes it clearer that
934+the variable is a module, and these object should be hidden anyway, since
935+they shouldn't be imported into other namespaces.
936+
937+While it is possible for ``lazy_import()`` to import members of a module
938+when using the ``from module import member`` syntax, it is recommended to
939+only use that syntax to load sub modules ``from module import submodule``.
940+This is because variables and classes can frequently be used without
941+needing a sub-member for example::
942+
943+ lazy_import(globals(), """
944+ from module import MyClass
945+ """)
946+
947+ def test(x):
948+ return isinstance(x, MyClass)
949+
950+This will incorrectly fail, because ``MyClass`` is a ``ImportReplacer``
951+object, rather than the real class.
952+
953+It also is incorrect to assign ``ImportReplacer`` objects to other variables.
954+Because the replacer only knows about the original name, it is unable to
955+replace other variables. The ``ImportReplacer`` class will raise an
956+``IllegalUseOfScopeReplacer`` exception if it can figure out that this
957+happened. But it requires accessing a member more than once from the new
958+variable, so some bugs are not detected right away.
959+
960+
961+The Null revision
962+=================
963+
964+The null revision is the ancestor of all revisions. Its revno is 0, its
965+revision-id is ``null:``, and its tree is the empty tree. When referring
966+to the null revision, please use ``bzrlib.revision.NULL_REVISION``. Old
967+code sometimes uses ``None`` for the null revision, but this practice is
968+being phased out.
969+
970+
971+Object string representations
972+=============================
973+
974+Python prints objects using their ``__repr__`` method when they are
975+written to logs, exception tracebacks, or the debugger. We want
976+objects to have useful representations to help in determining what went
977+wrong.
978+
979+If you add a new class you should generally add a ``__repr__`` method
980+unless there is an adequate method in a parent class. There should be a
981+test for the repr.
982+
983+Representations should typically look like Python constructor syntax, but
984+they don't need to include every value in the object and they don't need
985+to be able to actually execute. They're to be read by humans, not
986+machines. Don't hardcode the classname in the format, so that we get the
987+correct value if the method is inherited by a subclass. If you're
988+printing attributes of the object, including strings, you should normally
989+use ``%r`` syntax (to call their repr in turn).
990+
991+Try to avoid the representation becoming more than one or two lines long.
992+(But balance this against including useful information, and simplicity of
993+implementation.)
994+
995+Because repr methods are often called when something has already gone
996+wrong, they should be written somewhat more defensively than most code.
997+The object may be half-initialized or in some other way in an illegal
998+state. The repr method shouldn't raise an exception, or it may hide the
999+(probably more useful) underlying exception.
1000+
1001+Example::
1002+
1003+ def __repr__(self):
1004+ return '%s(%r)' % (self.__class__.__name__,
1005+ self._transport)
1006+
1007+
1008+Exception handling
1009+==================
1010+
1011+A bare ``except`` statement will catch all exceptions, including ones that
1012+really should terminate the program such as ``MemoryError`` and
1013+``KeyboardInterrupt``. They should rarely be used unless the exception is
1014+later re-raised. Even then, think about whether catching just
1015+``Exception`` (which excludes system errors in Python2.5 and later) would
1016+be better.
1017+
1018+
1019+Test coverage
1020+=============
1021+
1022+All code should be exercised by the test suite. See the `Bazaar Testing
1023+Guide <http://doc.bazaar-vcs.org/developers/testing.html>`_ for detailed
1024+information about writing tests.
1025+
1026+
1027+Assertions
1028+==========
1029+
1030+Do not use the Python ``assert`` statement, either in tests or elsewhere.
1031+A source test checks that it is not used. It is ok to explicitly raise
1032+AssertionError.
1033+
1034+Rationale:
1035+
1036+ * It makes the behaviour vary depending on whether bzr is run with -O
1037+ or not, therefore giving a chance for bugs that occur in one case or
1038+ the other, several of which have already occurred: assertions with
1039+ side effects, code which can't continue unless the assertion passes,
1040+ cases where we should give the user a proper message rather than an
1041+ assertion failure.
1042+ * It's not that much shorter than an explicit if/raise.
1043+ * It tends to lead to fuzzy thinking about whether the check is
1044+ actually needed or not, and whether it's an internal error or not
1045+ * It tends to cause look-before-you-leap patterns.
1046+ * It's unsafe if the check is needed to protect the integrity of the
1047+ user's data.
1048+ * It tends to give poor messages since the developer can get by with
1049+ no explanatory text at all.
1050+ * We can't rely on people always running with -O in normal use, so we
1051+ can't use it for tests that are actually expensive.
1052+ * Expensive checks that help developers are better turned on from the
1053+ test suite or a -D flag.
1054+ * If used instead of ``self.assert*()`` in tests it makes them falsely pass with -O.
1055+
1056+emacs setup
1057+===========
1058+
1059+In emacs::
1060+
1061+ ;(defface my-invalid-face
1062+ ; '((t (:background "Red" :underline t)))
1063+ ; "Face used to highlight invalid constructs or other uglyties"
1064+ ; )
1065+
1066+ (defun my-python-mode-hook ()
1067+ ;; setup preferred indentation style.
1068+ (setq fill-column 79)
1069+ (setq indent-tabs-mode nil) ; no tabs, never, I will not repeat
1070+ ; (font-lock-add-keywords 'python-mode
1071+ ; '(("^\\s *\t" . 'my-invalid-face) ; Leading tabs
1072+ ; ("[ \t]+$" . 'my-invalid-face) ; Trailing spaces
1073+ ; ("^[ \t]+$" . 'my-invalid-face)); Spaces only
1074+ ; )
1075+ )
1076+
1077+ (add-hook 'python-mode-hook 'my-python-mode-hook)
1078+
1079+The lines beginning with ';' are comments. They can be activated
1080+if one want to have a strong notice of some tab/space usage
1081+violations.
1082+
1083+Portability Tips
1084+================
1085+
1086+The ``bzrlib.osutils`` module has many useful helper functions, including
1087+some more portable variants of functions in the standard library.
1088+
1089+In particular, don't use ``shutil.rmtree`` unless it's acceptable for it
1090+to fail on Windows if some files are readonly or still open elsewhere.
1091+Use ``bzrlib.osutils.rmtree`` instead.
1092+
1093+
1094+
1095+..
1096+ vim: ft=rst tw=74 ai
1097
1098=== modified file 'doc/developers/index-plain.txt'
1099--- doc/developers/index-plain.txt 2010-05-12 11:04:21 +0000
1100+++ doc/developers/index-plain.txt 2010-05-23 20:46:24 +0000
1101@@ -26,6 +26,8 @@
1102
1103 * `Code Review <code-review.html>`_.
1104
1105+* `Bazaar Code Style Guide <code-style.html>`_.
1106+
1107 * `Writing plugins <http://doc.bazaar.canonical.com/plugins/en/plugin-development.html>`_
1108 |--| specific advice on writing Bazaar plugins. (web link)
1109
1110
1111=== modified file 'doc/developers/index.txt'
1112--- doc/developers/index.txt 2010-05-12 11:04:21 +0000
1113+++ doc/developers/index.txt 2010-05-23 20:46:24 +0000
1114@@ -24,6 +24,7 @@
1115 HACKING
1116 testing
1117 code-review
1118+ code-style
1119
1120 * `Contributing to Bazaar Documentation <http://wiki.bazaar.canonical.com/ContributingToTheDocs>`_ (wiki)
1121
1122
1123=== modified file 'doc/developers/integration.txt'
1124--- doc/developers/integration.txt 2009-09-15 06:07:11 +0000
1125+++ doc/developers/integration.txt 2010-05-23 20:46:24 +0000
1126@@ -2,8 +2,35 @@
1127 Integrating with Bazaar
1128 =======================
1129
1130-This page should hopefully become a quick guide to integrating other
1131-(Python-based) software with Bazaar.
1132+This document provides some general observations on integrating with
1133+Bazaar and some recipes for typical tasks. It is intended to be useful to
1134+someone developing either a plugin or some other piece of software that
1135+integrates with bzr. If you want to know about a topic that's not covered
1136+here, just ask us.
1137+
1138+
1139+
1140+
1141+Starting with bzrlib
1142+====================
1143+
1144+Before doing anything else with bzrlib, you should run `bzrlib.initialize`
1145+which sets up some global state.
1146+
1147+
1148+Running bzr commands
1149+====================
1150+
1151+To run command-line commands in-process::
1152+
1153+ from bzrlib.commands import get_command
1154+
1155+ cmd = get_command('version')
1156+ cmd.run([])
1157+
1158+This will send output through the current UIFactory; you can redirect this
1159+elsewhere through the parameters to `bzrlib.initialize`.
1160+
1161
1162 Manipulating the Working Tree
1163 =============================
1164@@ -21,7 +48,8 @@
1165 to see which methods are available.
1166
1167 Compare trees
1168-===============
1169+-------------
1170+
1171 There are two methods for comparing trees: ``changes_from`` and
1172 ``iter_changes``. ``iter_changes`` is more regular and precise, but it is
1173 somewhat harder to work with. See the API documentation for more details.
1174@@ -54,7 +82,7 @@
1175
1176
1177 Adding Files
1178-============
1179+------------
1180
1181 If you want to add files the same way ``bzr add`` does, you can use
1182 MutableTree.smart_add. By default, this is recursive. Paths can either be
1183@@ -70,7 +98,7 @@
1184
1185
1186 Removing Files
1187-==============
1188+--------------
1189
1190 You can remove multiple files at once. The file paths need to be relative
1191 to the workingtree::
1192@@ -85,7 +113,7 @@
1193
1194
1195 Renaming a File
1196-===============
1197+---------------
1198
1199 You can rename one file to a different name using WorkingTree.rename_one.
1200 You just provide the old and new names, eg::
1201@@ -94,7 +122,7 @@
1202
1203
1204 Moving Files
1205-============
1206+------------
1207
1208 You can move multiple files from one directory into another using
1209 WorkingTree.move::
1210@@ -107,7 +135,7 @@
1211
1212
1213 Committing Changes
1214-==================
1215+------------------
1216
1217 To commit _all_ the changes to our working tree we can just call the
1218 WorkingTree's commit method, giving it a commit message, eg::
1219@@ -187,7 +215,7 @@
1220
1221
1222 Branching from an existing branch
1223-=================================
1224+---------------------------------
1225
1226 To branch you create a branch object representing the branch you are
1227 branching from, and supply a path/url to the new branch location.
1228@@ -205,7 +233,7 @@
1229
1230
1231 Pushing and pulling branches
1232-============================
1233+----------------------------
1234
1235 To push a branch you need to open the source and destination branches, then
1236 just call push with the other branch as a parameter::
1237
1238=== modified file 'doc/developers/overview.txt'
1239--- doc/developers/overview.txt 2009-12-02 20:34:07 +0000
1240+++ doc/developers/overview.txt 2010-05-23 20:46:24 +0000
1241@@ -4,36 +4,20 @@
1242
1243 This document describes the key classes and concepts within Bazaar. It is
1244 intended to be useful to people working on the Bazaar codebase, or to
1245-people writing plugins.
1246+people writing plugins. People writing plugins may also like to read the
1247+guide to `Integrating with Bazaar <integrating.html>`_ for some specific
1248+recipes.
1249
1250 If you have any questions, or if something seems to be incorrect, unclear
1251 or missing, please talk to us in ``irc://irc.freenode.net/#bzr``, or write
1252-to the Bazaar mailing list. To propose a correction or addition to this
1253-document, send a merge request or new text to the mailing list.
1254-
1255-The current version of this document is available in the file
1256-``doc/developers/overview.txt`` in the source tree, and available online
1257-within the developer documentation, <http://doc.bazaar-vcs.org/developers/>.
1258-
1259-
1260-Essential Domain Classes
1261-########################
1262-
1263-The core domain objects within the bazaar model are:
1264-
1265-* Transport
1266-
1267-* Branch
1268-
1269-* Repository
1270-
1271-* WorkingTree
1272-
1273-Transports are explained below. See http://bazaar-vcs.org/Classes/
1274-for an introduction to the other key classes.
1275+to the Bazaar mailing list.
1276+
1277+
1278+Core classes
1279+############
1280
1281 Transport
1282-#########
1283+=========
1284
1285 The ``Transport`` layer handles access to local or remote directories.
1286 Each Transport object acts as a logical connection to a particular
1287@@ -46,7 +30,7 @@
1288 Python file I/O mechanisms.
1289
1290 Filenames vs URLs
1291-=================
1292+-----------------
1293
1294 Transports work in terms of URLs. Take note that URLs are by definition
1295 only ASCII - the decision of how to encode a Unicode string into a URL
1296@@ -83,14 +67,66 @@
1297 is also in the form of URL components.
1298
1299
1300+WorkingTree
1301+===========
1302+
1303+A workingtree is a special type of Tree that's associated with a working
1304+directory on disk, where the user can directly modify the files.
1305+
1306+Responsibilities:
1307+
1308+ * Maintaining a WorkingTree on disk at a file path.
1309+ * Maintaining the basis inventory (the inventory of the last commit done)
1310+ * Maintaining the working inventory.
1311+ * Maintaining the pending merges list.
1312+ * Maintaining the stat cache.
1313+ * Maintaining the last revision the working tree was updated to.
1314+ * Knows where its Branch is located.
1315+
1316+Dependencies:
1317+
1318+ * a Branch
1319+ * an MutableInventory
1320+ * local access to the working tree
1321+
1322+Branch
1323+======
1324+
1325+A Branch is a key user concept - its a single line of history that one or
1326+more people have been committing to.
1327+
1328+A Branch is responsible for:
1329+
1330+ * Holding user preferences that are set in a Branch.
1331+ * Holding the 'tip': the last revision to be committed to this Branch. (And the revno of that revision.)
1332+ * Knowing how to open the Repository that holds its history.
1333+ * Allowing write locks to be taken out to prevent concurrent alterations to the branch.
1334+
1335+Depends on:
1336+ * URL access to its base directory.
1337+ * A Transport to access its files.
1338+ * A Repository to hold its history.
1339+
1340 Repository
1341-##########
1342+==========
1343
1344 Repositories store committed history: file texts, revisions, inventories,
1345-and graph relationships between them.
1346+and graph relationships between them. A repository holds a bag of
1347+revision data that can be pointed to by various branches:
1348+
1349+ * Maintains storage of various history data at a URL:
1350+ * Revisions (Must have a matching inventory)
1351+ * Digital Signatures
1352+ * Inventories for each Revision. (Must have all the file texts available).
1353+ * File texts
1354+
1355+ * Synchronizes concurrent access to the repository by different
1356+ processes. (Most repository implementations use a physical
1357+ mutex only for a short period, and effectively support multiple readers
1358+ and writers.)
1359
1360 Stacked Repositories
1361-====================
1362+--------------------
1363
1364 A repository can be configured to refer to a list of "fallback"
1365 repositories. If a particular revision is not present in the original