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

[Martin Pool]

better docs on contributing, code reviews, and using bzrlib

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)

[Robert Collins (lifeless)]

Sure, looks nice. +1

[lp:~mbp/bzr/doc]

5236. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-17

(lifeless) Changes some tests that compare paths to use a fancier method
 than just string comparison, fixing them on Windows. (Martin [gz])

[Robert Collins (lifeless)]

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.

[Robert Collins (lifeless)]

review: Needs Fixing

[lp:~mbp/bzr/doc]

5237. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-18

(lifeless) Unbreak a win32 test that has a test method and attribute with
 the same label. (Martin [gz])

5238. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-18

(lifeless) Explicitly close the file handle in WT.get_file_text. (Tim Penhey)

5239. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-18

(Martin <gz>) Some cleanups for the zlib.crc32 changes

5240. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-19

(lifeless) Replace the unmaintained bzr completion script with gagern's new
 one. (Martin von Gagern)

5241. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-19

(parthm) Estimated records to be fetched are now shown for fetch (2a only).

5242. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-20

(andrew) Fix AttributeError in RemoteBranch.lock_write after
 lock_read. (#582781)

5243. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-20

(lifeless) Merge 2.1 into trunk with a number of fixes including pyrex 0.9.9
 support. (Robert Collins)

5244. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-20

(lifeless) Use the production Launchpad instance for XML-RPC requests (and
 also changes existing bug references to remove .edge from them. (Martin [gz])

5245. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-21

(lifeless) Merge from 2.1,
 fix for closing fd's when a specific file is supplied to status/commit etc.
 (Robert Collins)

5246. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-21

(spiv) Treat pywintypes.error as a user error as per OSError,
 rather than an internal error (Martin [gz])

5247. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-21

(lifeless) Switch lp-propose back to using edge. See bug 583667 for details.
 (Robert Collins)

5248. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-21

(lifeless) Add a --approve to lp-propose for proposals that are
 self-approvable. (Robert Collins)

5249. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-21

(lifeless) Fix deprecation warning in bash_completion plugin. (Andrew
 Bennetts)

5250. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-21

(lifeless) Some cleanups to the readdir pyrex code for a little efficiency
 and to avoid compile warnings. (John A Meinel)

[Robert Collins (lifeless)]

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.

[Robert Collins (lifeless)]

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.

[Martin Pool (mbp)]

> 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.

[Martin Pool (mbp)]

sent to pqm by email

[lp:~mbp/bzr/doc]

5251. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-23

(lifeless) Adjusted README.txt for bash_completion plugin,
 now that the plugin has been merged with the main tree. (Martin von Gagern)

5252. By Canonical.com Patch Queue Manager <email address hidden> on 2010-05-24

(mbp) Better docs on contributing, code reviews, and using bzrlib.

=== modified file 'doc/developers/HACKING.txt'
--- doc/developers/HACKING.txt	2010-05-12 11:04:21 +0000
+++ doc/developers/HACKING.txt	2010-05-23 20:46:24 +0000
@@ -35,8 +35,6 @@
 
 * Bug Tracker for the core product - https://bugs.launchpad.net/bzr/
 
-* Blueprint Tracker for the core product - https://blueprints.launchpad.net/bzr/
-
 If nothing else, perhaps you'll find inspiration in how other developers
 have solved their challenges.
 
@@ -128,6 +126,9 @@
 "Propose for merging into another branch".  Select "~bzr/bzr/trunk" to hand
 your changes off to the Bazaar developers for review and merging.
 
+Alternatively, after pushing you can use the ``lp-propose`` command to 
+create the merge proposal.
+
 Using a meaningful name for your branch will help you and the reviewer(s)
 better track the submission. Use a very succint description of your submission
 and prefix it with bug number if needed (lp:~mbp/bzr/484558-merge-directory
@@ -135,6 +136,30 @@
 (lp:~jameinel/bzr/export-file-511987).
 
 
+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.
+
+
 Why make a local copy of bzr.dev?
 ---------------------------------
 
@@ -270,363 +295,6 @@
 <http://doc.bazaar-vcs.org/developers/overview.html>`_.
 
 
-Coding Style Guidelines
-#######################
-
-hasattr and getattr
-===================
-
-``hasattr`` should not be used because it swallows exceptions including
-``KeyboardInterrupt``.  Instead, say something like ::
-
-  if getattr(thing, 'name', None) is None
-
-
-Code layout
-===========
-
-Please write PEP-8__ compliant code.
-
-__ http://www.python.org/peps/pep-0008.html
-
-One often-missed requirement is that the first line of docstrings
-should be a self-contained one-sentence summary.
-
-We use 4 space indents for blocks, and never use tab characters.  (In vim,
-``set expandtab``.)
-
-Trailing white space should be avoided, but is allowed.
-You should however not make lots of unrelated white space changes.
-
-Unix style newlines (LF) are used.
-
-Each file must have a newline at the end of it.
-
-Lines should be no more than 79 characters if at all possible.
-Lines that continue a long statement may be indented in either of
-two ways:
-
-within the parenthesis or other character that opens the block, e.g.::
-
-    my_long_method(arg1,
-                   arg2,
-                   arg3)
-
-or indented by four spaces::
-
-    my_long_method(arg1,
-        arg2,
-        arg3)
-
-The first is considered clearer by some people; however it can be a bit
-harder to maintain (e.g. when the method name changes), and it does not
-work well if the relevant parenthesis is already far to the right.  Avoid
-this::
-
-     self.legbone.kneebone.shinbone.toebone.shake_it(one,
-                                                     two,
-                                                     three)
-
-but rather ::
-
-     self.legbone.kneebone.shinbone.toebone.shake_it(one,
-         two,
-         three)
-
-or ::
-
-     self.legbone.kneebone.shinbone.toebone.shake_it(
-         one, two, three)
-
-For long lists, we like to add a trailing comma and put the closing
-character on the following line.  This makes it easier to add new items in
-future::
-
-    from bzrlib.goo import (
-        jam,
-        jelly,
-        marmalade,
-        )
-
-There should be spaces between function parameters, but not between the
-keyword name and the value::
-
-    call(1, 3, cheese=quark)
-
-In emacs::
-
-    ;(defface my-invalid-face
-    ;  '((t (:background "Red" :underline t)))
-    ;  "Face used to highlight invalid constructs or other uglyties"
-    ;  )
-
-    (defun my-python-mode-hook ()
-     ;; setup preferred indentation style.
-     (setq fill-column 79)
-     (setq indent-tabs-mode nil) ; no tabs, never, I will not repeat
-    ;  (font-lock-add-keywords 'python-mode
-    ;                         '(("^\\s *\t" . 'my-invalid-face) ; Leading tabs
-    ;                            ("[ \t]+$" . 'my-invalid-face)  ; Trailing spaces
-    ;                            ("^[ \t]+$" . 'my-invalid-face)); Spaces only
-    ;                          )
-     )
-
-    (add-hook 'python-mode-hook 'my-python-mode-hook)
-
-The lines beginning with ';' are comments. They can be activated
-if one want to have a strong notice of some tab/space usage
-violations.
-
-
-Module Imports
-==============
-
-* Imports should be done at the top-level of the file, unless there is
-  a strong reason to have them lazily loaded when a particular
-  function runs.  Import statements have a cost, so try to make sure
-  they don't run inside hot functions.
-
-* Module names should always be given fully-qualified,
-  i.e. ``bzrlib.hashcache`` not just ``hashcache``.
-
-
-Naming
-======
-
-Functions, methods or members that are relatively private are given
-a leading underscore prefix.  Names without a leading underscore are
-public not just across modules but to programmers using bzrlib as an
-API.
-
-We prefer class names to be concatenated capital words (``TestCase``)
-and variables, methods and functions to be lowercase words joined by
-underscores (``revision_id``, ``get_revision``).
-
-For the purposes of naming some names are treated as single compound
-words: "filename", "revno".
-
-Consider naming classes as nouns and functions/methods as verbs.
-
-Try to avoid using abbreviations in names, because there can be
-inconsistency if other people use the full name.
-
-
-Standard Names
-==============
-
-``revision_id`` not ``rev_id`` or ``revid``
-
-Functions that transform one thing to another should be named ``x_to_y``
-(not ``x2y`` as occurs in some old code.)
-
-
-Destructors
-===========
-
-Python destructors (``__del__``) work differently to those of other
-languages.  In particular, bear in mind that destructors may be called
-immediately when the object apparently becomes unreferenced, or at some
-later time, or possibly never at all.  Therefore we have restrictions on
-what can be done inside them.
-
- 0. If you think you need to use a ``__del__`` method ask another
-    developer for alternatives.  If you do need to use one, explain
-    why in a comment.
-
- 1. Never rely on a ``__del__`` method running.  If there is code that
-    must run, do it from a ``finally`` block instead.
-
- 2. Never ``import`` from inside a ``__del__`` method, or you may crash the
-    interpreter!!
-
- 3. In some places we raise a warning from the destructor if the object
-    has not been cleaned up or closed.  This is considered OK: the warning
-    may not catch every case but it's still useful sometimes.
-
-
-Cleanup methods
-===============
-
-Often when something has failed later code, including cleanups invoked
-from ``finally`` blocks, will fail too.  These secondary failures are
-generally uninteresting compared to the original exception.  So use the
-``only_raises`` decorator (from ``bzrlib.decorators``) for methods that
-are typically called in ``finally`` blocks, such as ``unlock`` methods.
-For example, ``@only_raises(LockNotHeld, LockBroken)``.  All errors that
-are unlikely to be a knock-on failure from a previous failure should be
-allowed.
-
-
-Factories
-=========
-
-In some places we have variables which point to callables that construct
-new instances.  That is to say, they can be used a lot like class objects,
-but they shouldn't be *named* like classes::
-
-> I think that things named FooBar should create instances of FooBar when
-> called. Its plain confusing for them to do otherwise. When we have
-> something that is going to be used as a class - that is, checked for via
-> isinstance or other such idioms, them I would call it foo_class, so that
-> it is clear that a callable is not sufficient. If it is only used as a
-> factory, then yes, foo_factory is what I would use.
-
-
-Registries
-==========
-
-Several places in Bazaar use (or will use) a registry, which is a
-mapping from names to objects or classes.  The registry allows for
-loading in registered code only when it's needed, and keeping
-associated information such as a help string or description.
-
-
-InterObject and multiple dispatch
-=================================
-
-The ``InterObject`` provides for two-way `multiple dispatch`__: matching
-up for example a source and destination repository to find the right way
-to transfer data between them.
-
-.. __: http://en.wikipedia.org/wiki/Multiple_dispatch
-
-There is a subclass ``InterObject`` classes for each type of object that is
-dispatched this way, e.g. ``InterRepository``.  Calling ``.get()`` on this
-class will return an ``InterObject`` instance providing the best match for
-those parameters, and this instance then has methods for operations
-between the objects.
-
-::
-
-  inter = InterRepository.get(source_repo, target_repo)
-  inter.fetch(revision_id)
-
-``InterRepository`` also acts as a registry-like object for its
-subclasses, and they can be added through ``.register_optimizer``.  The
-right one to run is selected by asking each class, in reverse order of
-registration, whether it ``.is_compatible`` with the relevant objects.
-
-Lazy Imports
-============
-
-To make startup time faster, we use the ``bzrlib.lazy_import`` module to
-delay importing modules until they are actually used. ``lazy_import`` uses
-the same syntax as regular python imports. So to import a few modules in a
-lazy fashion do::
-
-  from bzrlib.lazy_import import lazy_import
-  lazy_import(globals(), """
-  import os
-  import subprocess
-  import sys
-  import time
-
-  from bzrlib import (
-     errors,
-     transport,
-     revision as _mod_revision,
-     )
-  import bzrlib.transport
-  import bzrlib.xml5
-  """)
-
-At this point, all of these exist as a ``ImportReplacer`` object, ready to
-be imported once a member is accessed. Also, when importing a module into
-the local namespace, which is likely to clash with variable names, it is
-recommended to prefix it as ``_mod_<module>``. This makes it clearer that
-the variable is a module, and these object should be hidden anyway, since
-they shouldn't be imported into other namespaces.
-
-While it is possible for ``lazy_import()`` to import members of a module
-when using the ``from module import member`` syntax, it is recommended to
-only use that syntax to load sub modules ``from module import submodule``.
-This is because variables and classes can frequently be used without
-needing a sub-member for example::
-
-  lazy_import(globals(), """
-  from module import MyClass
-  """)
-
-  def test(x):
-      return isinstance(x, MyClass)
-
-This will incorrectly fail, because ``MyClass`` is a ``ImportReplacer``
-object, rather than the real class.
-
-It also is incorrect to assign ``ImportReplacer`` objects to other variables.
-Because the replacer only knows about the original name, it is unable to
-replace other variables. The ``ImportReplacer`` class will raise an
-``IllegalUseOfScopeReplacer`` exception if it can figure out that this
-happened. But it requires accessing a member more than once from the new
-variable, so some bugs are not detected right away.
-
-
-The Null revision
-=================
-
-The null revision is the ancestor of all revisions.  Its revno is 0, its
-revision-id is ``null:``, and its tree is the empty tree.  When referring
-to the null revision, please use ``bzrlib.revision.NULL_REVISION``.  Old
-code sometimes uses ``None`` for the null revision, but this practice is
-being phased out.
-
-
-Object string representations
-=============================
-
-Python prints objects using their ``__repr__`` method when they are
-written to logs, exception tracebacks, or the debugger.  We want
-objects to have useful representations to help in determining what went
-wrong.
-
-If you add a new class you should generally add a ``__repr__`` method
-unless there is an adequate method in a parent class.  There should be a
-test for the repr.
-
-Representations should typically look like Python constructor syntax, but
-they don't need to include every value in the object and they don't need
-to be able to actually execute.  They're to be read by humans, not
-machines.  Don't hardcode the classname in the format, so that we get the
-correct value if the method is inherited by a subclass.  If you're
-printing attributes of the object, including strings, you should normally
-use ``%r`` syntax (to call their repr in turn).
-
-Try to avoid the representation becoming more than one or two lines long.
-(But balance this against including useful information, and simplicity of
-implementation.)
-
-Because repr methods are often called when something has already gone
-wrong, they should be written somewhat more defensively than most code.
-The object may be half-initialized or in some other way in an illegal
-state.  The repr method shouldn't raise an exception, or it may hide the
-(probably more useful) underlying exception.
-
-Example::
-
-    def __repr__(self):
-        return '%s(%r)' % (self.__class__.__name__,
-                           self._transport)
-
-
-Exception handling
-==================
-
-A bare ``except`` statement will catch all exceptions, including ones that
-really should terminate the program such as ``MemoryError`` and
-``KeyboardInterrupt``.  They should rarely be used unless the exception is
-later re-raised.  Even then, think about whether catching just
-``Exception`` (which excludes system errors in Python2.5 and later) would
-be better.
-
-
-Test coverage
-=============
-
-All code should be exercised by the test suite.  See the `Bazaar Testing
-Guide <http://doc.bazaar-vcs.org/developers/testing.html>`_ for detailed
-information about writing tests.
-
-
 Core Topics
 ###########
 
@@ -891,35 +559,6 @@
 final fullstop.  If long, they may contain newlines to break the text.
 
 
-Assertions
-==========
-
-Do not use the Python ``assert`` statement, either in tests or elsewhere.
-A source test checks that it is not used.  It is ok to explicitly raise
-AssertionError.
-
-Rationale:
-
- * It makes the behaviour vary depending on whether bzr is run with -O
-   or not, therefore giving a chance for bugs that occur in one case or
-   the other, several of which have already occurred: assertions with
-   side effects, code which can't continue unless the assertion passes,
-   cases where we should give the user a proper message rather than an
-   assertion failure.
- * It's not that much shorter than an explicit if/raise.
- * It tends to lead to fuzzy thinking about whether the check is
-   actually needed or not, and whether it's an internal error or not
- * It tends to cause look-before-you-leap patterns.
- * It's unsafe if the check is needed to protect the integrity of the
-   user's data.
- * It tends to give poor messages since the developer can get by with
-   no explanatory text at all.
- * We can't rely on people always running with -O in normal use, so we
-   can't use it for tests that are actually expensive.
- * Expensive checks that help developers are better turned on from the
-   test suite or a -D flag.
- * If used instead of ``self.assert*()`` in tests it makes them falsely pass with -O.
-
 
 Documenting Changes
 ===================
@@ -1143,17 +782,6 @@
 valid characters are generated where possible.
 
 
-Portability Tips
-================
-
-The ``bzrlib.osutils`` module has many useful helper functions, including
-some more portable variants of functions in the standard library.
-
-In particular, don't use ``shutil.rmtree`` unless it's acceptable for it
-to fail on Windows if some files are readonly or still open elsewhere.
-Use ``bzrlib.osutils.rmtree`` instead.
-
-
 C Extension Modules
 ===================
 
@@ -1411,44 +1039,10 @@
 results.
 
 
-Reviewing Blueprints
-====================
-
-Blueprint Tracking Using Launchpad
-----------------------------------
-
-New features typically require a fair amount of discussion, design and
-debate. For Bazaar, that information is often captured in a so-called
-"blueprint" on our Wiki. Overall tracking of blueprints and their status
-is done using Launchpad's relevant tracker,
-https://blueprints.launchpad.net/bzr/. Once a blueprint for ready for
-review, please announce it on the mailing list.
-
-Alternatively, send an email beginning with [RFC] with the proposal to the
-list. In some cases, you may wish to attach proposed code  or a proposed
-developer document if that best communicates the idea. Debate can then
-proceed using the normal merge review processes.
-
-
-Recording Blueprint Review Feedback
------------------------------------
-
-Unlike its Bug Tracker, Launchpad's Blueprint Tracker doesn't currently
-(Jun 2007) support a chronological list of comment responses. Review
-feedback can either be recorded on the Wiki hosting the blueprints or by
-using Launchpad's whiteboard feature.
-
-
 Planning Releases
 =================
 
 
-Using Releases and Milestones in Launchpad
-------------------------------------------
-
-TODO ... (Exact policies still under discussion)
-
-
 Bug Triage
 ----------
 

=== modified file 'doc/developers/code-review.txt'
--- doc/developers/code-review.txt	2010-05-12 11:04:21 +0000
+++ doc/developers/code-review.txt	2010-05-23 20:46:24 +0000
@@ -1,55 +1,28 @@
-The Code Review Process
-#######################
-
-All code changes coming in to Bazaar are reviewed by someone else.
+Reviewing proposed changes to Bazaar
+####################################
+
+All non-trivial code changes coming in to Bazaar are reviewed by someone else.
+
+Anyone is welcome to review any patch.  You don't need to have a full 
+understanding of the codebase to find problems in the code, the documentation,
+or the concept of the patch.
+
 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.
+developers.  Use intelligent discretion about whether if the patch is trivial.
+
+No one likes their merge requests sitting in a queue going nowhere: this 
+is pure waste.  We prioritize reviewing existing proposals.  
+Canonical dedicates some staff time to providing prompt helpful reviews.  
+(See <http://wiki.bazaar.canonical.com/PatchPilot/>.)
+
+From late 2009 on, we do all our code reviews through Launchpad's 
+merge proposal interface.
 
 
 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,
@@ -72,7 +45,7 @@
 
 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.
+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
@@ -109,62 +82,23 @@
 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 ``[e-mail]``.
-
-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.
 
+<https://help.launchpad.net/Code/Review> explains the various merge proposal
+states.  Note that we don't use state *Approved* until the patch is completely
+ready to merge.
+
+
+Landing approved changes
+========================
+
+Once a merge proposal is approved and finished, it's sent to PQM (the patch
+queue manager) which will automatically test and integrate it.  The recommended
+way to start this off is by running the ``feed-pqm`` script from 
+<https://launchpad.net/hydrazine/>.

=== added file 'doc/developers/code-style.txt'
--- doc/developers/code-style.txt	1970-01-01 00:00:00 +0000
+++ doc/developers/code-style.txt	2010-05-23 20:46:24 +0000
@@ -0,0 +1,413 @@
+***********************
+Bazaar Code Style Guide
+***********************
+
+Code layout
+===========
+
+Please write PEP-8__ compliant code.
+
+__ http://www.python.org/peps/pep-0008.html
+
+One often-missed requirement is that the first line of docstrings
+should be a self-contained one-sentence summary.
+
+We use 4 space indents for blocks, and never use tab characters.  (In vim,
+``set expandtab``.)
+
+Trailing white space should be avoided, but is allowed.
+You should however not make lots of unrelated white space changes.
+
+Unix style newlines (LF) are used.
+
+Each file must have a newline at the end of it.
+
+Lines should be no more than 79 characters if at all possible.
+Lines that continue a long statement may be indented in either of
+two ways:
+
+within the parenthesis or other character that opens the block, e.g.::
+
+    my_long_method(arg1,
+                   arg2,
+                   arg3)
+
+or indented by four spaces::
+
+    my_long_method(arg1,
+        arg2,
+        arg3)
+
+The first is considered clearer by some people; however it can be a bit
+harder to maintain (e.g. when the method name changes), and it does not
+work well if the relevant parenthesis is already far to the right.  Avoid
+this::
+
+     self.legbone.kneebone.shinbone.toebone.shake_it(one,
+                                                     two,
+                                                     three)
+
+but rather ::
+
+     self.legbone.kneebone.shinbone.toebone.shake_it(one,
+         two,
+         three)
+
+or ::
+
+     self.legbone.kneebone.shinbone.toebone.shake_it(
+         one, two, three)
+
+For long lists, we like to add a trailing comma and put the closing
+character on the following line.  This makes it easier to add new items in
+future::
+
+    from bzrlib.goo import (
+        jam,
+        jelly,
+        marmalade,
+        )
+
+There should be spaces between function parameters, but not between the
+keyword name and the value::
+
+    call(1, 3, cheese=quark)
+
+
+Python versions
+===============
+
+Bazaar supports Python from 2.4 through 2.6, and in the future we want to
+support Python 3.0.  Avoid using language features added in 2.5 or 2.6, or
+features deprecated in Python 3.0.  (You can check v3 compatibility using
+the ``-3`` option of Python2.6.)
+
+Specifically:
+
+ * don't use the ``with`` statement
+ * don't ``from . import``
+
+
+hasattr and getattr
+===================
+
+``hasattr`` should not be used because it swallows exceptions including
+``KeyboardInterrupt``.  Instead, say something like ::
+
+  if getattr(thing, 'name', None) is None
+
+
+Module Imports
+==============
+
+* Imports should be done at the top-level of the file, unless there is
+  a strong reason to have them lazily loaded when a particular
+  function runs.  Import statements have a cost, so try to make sure
+  they don't run inside hot functions.
+
+* Module names should always be given fully-qualified,
+  i.e. ``bzrlib.hashcache`` not just ``hashcache``.
+
+
+Naming
+======
+
+Functions, methods or members that are relatively private are given
+a leading underscore prefix.  Names without a leading underscore are
+public not just across modules but to programmers using bzrlib as an
+API.
+
+We prefer class names to be concatenated capital words (``TestCase``)
+and variables, methods and functions to be lowercase words joined by
+underscores (``revision_id``, ``get_revision``).
+
+For the purposes of naming some names are treated as single compound
+words: "filename", "revno".
+
+Consider naming classes as nouns and functions/methods as verbs.
+
+Try to avoid using abbreviations in names, because there can be
+inconsistency if other people use the full name.
+
+
+Standard Names
+==============
+
+``revision_id`` not ``rev_id`` or ``revid``
+
+Functions that transform one thing to another should be named ``x_to_y``
+(not ``x2y`` as occurs in some old code.)
+
+
+Destructors
+===========
+
+Python destructors (``__del__``) work differently to those of other
+languages.  In particular, bear in mind that destructors may be called
+immediately when the object apparently becomes unreferenced, or at some
+later time, or possibly never at all.  Therefore we have restrictions on
+what can be done inside them.
+
+ 0. If you think you need to use a ``__del__`` method ask another
+    developer for alternatives.  If you do need to use one, explain
+    why in a comment.
+
+ 1. Never rely on a ``__del__`` method running.  If there is code that
+    must run, do it from a ``finally`` block instead.
+
+ 2. Never ``import`` from inside a ``__del__`` method, or you may crash the
+    interpreter!!
+
+ 3. In some places we raise a warning from the destructor if the object
+    has not been cleaned up or closed.  This is considered OK: the warning
+    may not catch every case but it's still useful sometimes.
+
+
+Cleanup methods
+===============
+
+Often when something has failed later code, including cleanups invoked
+from ``finally`` blocks, will fail too.  These secondary failures are
+generally uninteresting compared to the original exception.  So use the
+``only_raises`` decorator (from ``bzrlib.decorators``) for methods that
+are typically called in ``finally`` blocks, such as ``unlock`` methods.
+For example, ``@only_raises(LockNotHeld, LockBroken)``.  All errors that
+are unlikely to be a knock-on failure from a previous failure should be
+allowed.
+
+
+Factories
+=========
+
+In some places we have variables which point to callables that construct
+new instances.  That is to say, they can be used a lot like class objects,
+but they shouldn't be *named* like classes.  Things called ``FooBar`` should
+create an instance of ``FooBar``.  A factory method that might create a
+``FooBar`` or might make something else should be called ``foo_factory``.
+
+
+Registries
+==========
+
+Several places in Bazaar use (or will use) a registry, which is a
+mapping from names to objects or classes.  The registry allows for
+loading in registered code only when it's needed, and keeping
+associated information such as a help string or description.
+
+
+InterObject and multiple dispatch
+=================================
+
+The ``InterObject`` provides for two-way `multiple dispatch`__: matching
+up for example a source and destination repository to find the right way
+to transfer data between them.
+
+.. __: http://en.wikipedia.org/wiki/Multiple_dispatch
+
+There is a subclass ``InterObject`` classes for each type of object that is
+dispatched this way, e.g. ``InterRepository``.  Calling ``.get()`` on this
+class will return an ``InterObject`` instance providing the best match for
+those parameters, and this instance then has methods for operations
+between the objects.
+
+::
+
+  inter = InterRepository.get(source_repo, target_repo)
+  inter.fetch(revision_id)
+
+``InterRepository`` also acts as a registry-like object for its
+subclasses, and they can be added through ``.register_optimizer``.  The
+right one to run is selected by asking each class, in reverse order of
+registration, whether it ``.is_compatible`` with the relevant objects.
+
+Lazy Imports
+============
+
+To make startup time faster, we use the ``bzrlib.lazy_import`` module to
+delay importing modules until they are actually used. ``lazy_import`` uses
+the same syntax as regular python imports. So to import a few modules in a
+lazy fashion do::
+
+  from bzrlib.lazy_import import lazy_import
+  lazy_import(globals(), """
+  import os
+  import subprocess
+  import sys
+  import time
+
+  from bzrlib import (
+     errors,
+     transport,
+     revision as _mod_revision,
+     )
+  import bzrlib.transport
+  import bzrlib.xml5
+  """)
+
+At this point, all of these exist as a ``ImportReplacer`` object, ready to
+be imported once a member is accessed. Also, when importing a module into
+the local namespace, which is likely to clash with variable names, it is
+recommended to prefix it as ``_mod_<module>``. This makes it clearer that
+the variable is a module, and these object should be hidden anyway, since
+they shouldn't be imported into other namespaces.
+
+While it is possible for ``lazy_import()`` to import members of a module
+when using the ``from module import member`` syntax, it is recommended to
+only use that syntax to load sub modules ``from module import submodule``.
+This is because variables and classes can frequently be used without
+needing a sub-member for example::
+
+  lazy_import(globals(), """
+  from module import MyClass
+  """)
+
+  def test(x):
+      return isinstance(x, MyClass)
+
+This will incorrectly fail, because ``MyClass`` is a ``ImportReplacer``
+object, rather than the real class.
+
+It also is incorrect to assign ``ImportReplacer`` objects to other variables.
+Because the replacer only knows about the original name, it is unable to
+replace other variables. The ``ImportReplacer`` class will raise an
+``IllegalUseOfScopeReplacer`` exception if it can figure out that this
+happened. But it requires accessing a member more than once from the new
+variable, so some bugs are not detected right away.
+
+
+The Null revision
+=================
+
+The null revision is the ancestor of all revisions.  Its revno is 0, its
+revision-id is ``null:``, and its tree is the empty tree.  When referring
+to the null revision, please use ``bzrlib.revision.NULL_REVISION``.  Old
+code sometimes uses ``None`` for the null revision, but this practice is
+being phased out.
+
+
+Object string representations
+=============================
+
+Python prints objects using their ``__repr__`` method when they are
+written to logs, exception tracebacks, or the debugger.  We want
+objects to have useful representations to help in determining what went
+wrong.
+
+If you add a new class you should generally add a ``__repr__`` method
+unless there is an adequate method in a parent class.  There should be a
+test for the repr.
+
+Representations should typically look like Python constructor syntax, but
+they don't need to include every value in the object and they don't need
+to be able to actually execute.  They're to be read by humans, not
+machines.  Don't hardcode the classname in the format, so that we get the
+correct value if the method is inherited by a subclass.  If you're
+printing attributes of the object, including strings, you should normally
+use ``%r`` syntax (to call their repr in turn).
+
+Try to avoid the representation becoming more than one or two lines long.
+(But balance this against including useful information, and simplicity of
+implementation.)
+
+Because repr methods are often called when something has already gone
+wrong, they should be written somewhat more defensively than most code.
+The object may be half-initialized or in some other way in an illegal
+state.  The repr method shouldn't raise an exception, or it may hide the
+(probably more useful) underlying exception.
+
+Example::
+
+    def __repr__(self):
+        return '%s(%r)' % (self.__class__.__name__,
+                           self._transport)
+
+
+Exception handling
+==================
+
+A bare ``except`` statement will catch all exceptions, including ones that
+really should terminate the program such as ``MemoryError`` and
+``KeyboardInterrupt``.  They should rarely be used unless the exception is
+later re-raised.  Even then, think about whether catching just
+``Exception`` (which excludes system errors in Python2.5 and later) would
+be better.
+
+
+Test coverage
+=============
+
+All code should be exercised by the test suite.  See the `Bazaar Testing
+Guide <http://doc.bazaar-vcs.org/developers/testing.html>`_ for detailed
+information about writing tests.
+
+
+Assertions
+==========
+
+Do not use the Python ``assert`` statement, either in tests or elsewhere.
+A source test checks that it is not used.  It is ok to explicitly raise
+AssertionError.
+
+Rationale:
+
+ * It makes the behaviour vary depending on whether bzr is run with -O
+   or not, therefore giving a chance for bugs that occur in one case or
+   the other, several of which have already occurred: assertions with
+   side effects, code which can't continue unless the assertion passes,
+   cases where we should give the user a proper message rather than an
+   assertion failure.
+ * It's not that much shorter than an explicit if/raise.
+ * It tends to lead to fuzzy thinking about whether the check is
+   actually needed or not, and whether it's an internal error or not
+ * It tends to cause look-before-you-leap patterns.
+ * It's unsafe if the check is needed to protect the integrity of the
+   user's data.
+ * It tends to give poor messages since the developer can get by with
+   no explanatory text at all.
+ * We can't rely on people always running with -O in normal use, so we
+   can't use it for tests that are actually expensive.
+ * Expensive checks that help developers are better turned on from the
+   test suite or a -D flag.
+ * If used instead of ``self.assert*()`` in tests it makes them falsely pass with -O.
+
+emacs setup
+===========
+
+In emacs::
+
+    ;(defface my-invalid-face
+    ;  '((t (:background "Red" :underline t)))
+    ;  "Face used to highlight invalid constructs or other uglyties"
+    ;  )
+
+    (defun my-python-mode-hook ()
+     ;; setup preferred indentation style.
+     (setq fill-column 79)
+     (setq indent-tabs-mode nil) ; no tabs, never, I will not repeat
+    ;  (font-lock-add-keywords 'python-mode
+    ;                         '(("^\\s *\t" . 'my-invalid-face) ; Leading tabs
+    ;                            ("[ \t]+$" . 'my-invalid-face)  ; Trailing spaces
+    ;                            ("^[ \t]+$" . 'my-invalid-face)); Spaces only
+    ;                          )
+     )
+
+    (add-hook 'python-mode-hook 'my-python-mode-hook)
+
+The lines beginning with ';' are comments. They can be activated
+if one want to have a strong notice of some tab/space usage
+violations.
+
+Portability Tips
+================
+
+The ``bzrlib.osutils`` module has many useful helper functions, including
+some more portable variants of functions in the standard library.
+
+In particular, don't use ``shutil.rmtree`` unless it's acceptable for it
+to fail on Windows if some files are readonly or still open elsewhere.
+Use ``bzrlib.osutils.rmtree`` instead.
+
+
+
+..
+   vim: ft=rst tw=74 ai

=== modified file 'doc/developers/index-plain.txt'
--- doc/developers/index-plain.txt	2010-05-12 11:04:21 +0000
+++ doc/developers/index-plain.txt	2010-05-23 20:46:24 +0000
@@ -26,6 +26,8 @@
 
 * `Code Review <code-review.html>`_.
 
+* `Bazaar Code Style Guide <code-style.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'
--- doc/developers/index.txt	2010-05-12 11:04:21 +0000
+++ doc/developers/index.txt	2010-05-23 20:46:24 +0000
@@ -24,6 +24,7 @@
    HACKING
    testing
    code-review
+   code-style
 
 * `Contributing to Bazaar Documentation <http://wiki.bazaar.canonical.com/ContributingToTheDocs>`_ (wiki)
 

=== modified file 'doc/developers/integration.txt'
--- doc/developers/integration.txt	2009-09-15 06:07:11 +0000
+++ doc/developers/integration.txt	2010-05-23 20:46:24 +0000
@@ -2,8 +2,35 @@
 Integrating with Bazaar
 =======================
 
-This page should hopefully become a quick guide to integrating other
-(Python-based) software with Bazaar.
+This document provides some general observations on integrating with
+Bazaar and some recipes for typical tasks.  It is intended to be useful to
+someone developing either a plugin or some other piece of software that
+integrates with bzr.  If you want to know about a topic that's not covered
+here, just ask us.
+
+
+
+
+Starting with bzrlib
+====================
+
+Before doing anything else with bzrlib, you should run `bzrlib.initialize`
+which sets up some global state.  
+
+
+Running bzr commands
+====================
+
+To run command-line commands in-process::
+
+  from bzrlib.commands import get_command
+  
+  cmd = get_command('version')
+  cmd.run([])
+  
+This will send output through the current UIFactory; you can redirect this
+elsewhere through the parameters to `bzrlib.initialize`.
+
 
 Manipulating the Working Tree
 =============================
@@ -21,7 +48,8 @@
 to see which methods are available.
 
 Compare trees
-===============
+-------------
+
 There are two methods for comparing trees: ``changes_from`` and
 ``iter_changes``.  ``iter_changes`` is more regular and precise, but it is
 somewhat harder to work with.  See the API documentation for more details.
@@ -54,7 +82,7 @@
 
 
 Adding Files
-============
+------------
 
 If you want to add files the same way ``bzr add`` does, you can use
 MutableTree.smart_add.  By default, this is recursive. Paths can either be
@@ -70,7 +98,7 @@
 
 
 Removing Files
-==============
+--------------
 
 You can remove multiple files at once.  The file paths need to be relative
 to the workingtree::
@@ -85,7 +113,7 @@
 
 
 Renaming a File
-===============
+---------------
 
 You can rename one file to a different name using WorkingTree.rename_one.
 You just provide the old and new names, eg::
@@ -94,7 +122,7 @@
 
 
 Moving Files
-============
+------------
 
 You can move multiple files from one directory into another using
 WorkingTree.move::
@@ -107,7 +135,7 @@
 
 
 Committing Changes
-==================
+------------------
 
 To commit _all_ the changes to our working tree we can just call the
 WorkingTree's commit method, giving it a commit message, eg::
@@ -187,7 +215,7 @@
 
 
 Branching from an existing branch
-=================================
+---------------------------------
 
 To branch you create a branch object representing the branch you are
 branching from, and supply a path/url to the new branch location.
@@ -205,7 +233,7 @@
 
 
 Pushing and pulling branches
-============================
+----------------------------
 
 To push a branch you need to open the source and destination branches, then
 just call push with the other branch as a parameter::

=== modified file 'doc/developers/overview.txt'
--- doc/developers/overview.txt	2009-12-02 20:34:07 +0000
+++ doc/developers/overview.txt	2010-05-23 20:46:24 +0000
@@ -4,36 +4,20 @@
 
 This document describes the key classes and concepts within Bazaar.  It is
 intended to be useful to people working on the Bazaar codebase, or to
-people writing plugins.
+people writing plugins.  People writing plugins may also like to read the 
+guide to `Integrating with Bazaar <integrating.html>`_ for some specific
+recipes.
 
 If you have any questions, or if something seems to be incorrect, unclear
 or missing, please talk to us in ``irc://irc.freenode.net/#bzr``, or write
-to the Bazaar mailing list.  To propose a correction or addition to this
-document, send a merge request or new text to the mailing list.
-
-The current version of this document is available in the file
-``doc/developers/overview.txt`` in the source tree, and available online
-within the developer documentation, <http://doc.bazaar-vcs.org/developers/>.
-
-
-Essential Domain Classes
-########################
-
-The core domain objects within the bazaar model are:
-
-* Transport
-
-* Branch
-
-* Repository
-
-* WorkingTree
-
-Transports are explained below. See http://bazaar-vcs.org/Classes/
-for an introduction to the other key classes.
+to the Bazaar mailing list.  
+
+
+Core classes
+############
 
 Transport
-#########
+=========
 
 The ``Transport`` layer handles access to local or remote directories.
 Each Transport object acts as a logical connection to a particular
@@ -46,7 +30,7 @@
 Python file I/O mechanisms.
 
 Filenames vs URLs
-=================
+-----------------
 
 Transports work in terms of URLs.  Take note that URLs are by definition
 only ASCII - the decision of how to encode a Unicode string into a URL
@@ -83,14 +67,66 @@
 is also in the form of URL components.
 
 
+WorkingTree
+===========
+
+A workingtree is a special type of Tree that's associated with a working
+directory on disk, where the user can directly modify the files. 
+
+Responsibilities:
+
+ * Maintaining a WorkingTree on disk at a file path.
+ * Maintaining the basis inventory (the inventory of the last commit done)
+ * Maintaining the working inventory.
+ * Maintaining the pending merges list.
+ * Maintaining the stat cache.
+ * Maintaining the last revision the working tree was updated to.
+ * Knows where its Branch is located.
+
+Dependencies:
+
+ * a Branch
+ * an MutableInventory
+ * local access to the working tree
+
+Branch
+======
+
+A Branch is a key user concept - its a single line of history that one or
+more people have been committing to. 
+
+A Branch is responsible for:
+
+ * Holding user preferences that are set in a Branch.
+ * Holding the 'tip': the last revision to be committed to this Branch. (And the revno of that revision.)
+ * Knowing how to open the Repository that holds its history.
+ * Allowing write locks to be taken out to prevent concurrent alterations to the branch.
+
+Depends on:
+ * URL access to its base directory.
+ * A Transport to access its files.
+ * A Repository to hold its history.
+
 Repository
-##########
+==========
 
 Repositories store committed history: file texts, revisions, inventories,
-and graph relationships between them.
+and graph relationships between them.  A repository holds a bag of
+revision data that can be pointed to by various branches:
+
+ * Maintains storage of various history data at a URL:
+   * Revisions (Must have a matching inventory)
+   * Digital Signatures
+   * Inventories for each Revision. (Must have all the file texts available).
+   * File texts
+
+ * Synchronizes concurrent access to the repository by different
+   processes.  (Most repository implementations use a physical 
+   mutex only for a short period, and effectively support multiple readers
+   and writers.)
 
 Stacked Repositories
-====================
+--------------------
 
 A repository can be configured to refer to a list of "fallback"
 repositories.  If a particular revision is not present in the original