[MERGE] bzr-dir phase 2

[Robert Collins]

On Sun, 2006-02-12 at 12:54 -0600, John A Meinel wrote:

> I'm starting to go through it, so here goes...
> 
> What is the use case for get_branch_transport() based on branch_format?

As the docstring says, this provides the branch transport for use in
that bzrdir. Different bzr dirs may be compatible with different branch
formats, passing the branch format in during initialization allows the
bzrdir to check for compatability - and we use that to ensure that
format-string-less branch formats (that is branch data format 4, the
current format) do not get used with meta-dir environments and vice
verca.

> The parameter for get_workingtree_transport() is labeled 'branch_format'
> not 'workingtree_format'.

Doh. Fixed.

> Should __init__() always be the first entry in a class?

I've been putting methods in alphabetical order ignoring __'s. But I can
change happily.

> You almost use epydoc format for the doc strings, but it would be:
> 
> :param _format: stuff
> 
> rather than just
> _format: stuff

I'll tweak the ones I spot, but I'm not too concerned unless you are.

> 'BzrDir.open()' says "Open an existing *branch*, rooted at 'base'.

Fixed.

> We probably shouldn't be raising NotBranchError from
> BzrDir.open_containing(). We should raise NotBzrError, or somesuch.

I thought it was best for code compatability, but I'm happy to change
it.

> Why does 'sprout()' always create a standalone branch?

At this point in time I dont have any [implementable] use cases for
making a new line of development but not making a working tree.
Repositories are not implemented, and the UI has no concept of 'branch
--no-working-tree' or some such.

> Why does get_repository_transport() create the 'repository' directory?

Same reason get_branch_transport creates the 'branch' directory - the
bzrdir is setting up the environment for the repository format to use.
It seemed cleaner to me to have the repository just say 'give me a ready
to use transport' than to have every repository format need to fiddle
with mkdir etc. Particularly as the repository is path agnostic - it
does not know where it is based except via having a transport connection
to it.

> You have 'BzrDirFormat5' having format 4 branches, and format 6
> repositories (always). Shouldn't that be format 5 repositories? (I think
> this is just a documentation bug.)

Fixed.

> 
> 10k lines is a lot of code to review. But your results do speak for
> themselves. (I didn't get through everything, but I tried).

Thanks!

Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060213/84a57a07/attachment.pgp