[PATCH] cleanup branch.find_branch_root() (was Re: About merging)

[John A Meinel]

David Vanderson wrote:

> Hello,
>
> Been lurking on the list for quite some time.  Here's my first go at a
> simple patch based on what Aaron said:
>
> Aaron Bentley wrote:
> <snip>
>
>> One easy-but-boring thing would be factoring out the options for the
>> Branch constructor.
>>
>> Branch.__init__ currently takes the options "init" and "find_root".
>> Their functionality should be separated out into factory functions, so
>> that we get init_branch() and find_root_branch().
>>
>> Aaron
>
>
> <snip>
>
> The attached patch (off of revision 763) removes the "find_root"
> option by making it always happen unless "init" is True.  Turns out
> there was no code that wanted find_root to be False.
>
> Please tell me if you see any problems, especially as this is my first
> patch.
>
> Thanks,
> Dave


I think the preferred fix is to make a Branch() object always require an
explicit base, which must exist, and must be the root. And then change
all of the other places in the code to use "find_branch".

Admittedly, the current code always wants to search, but in my mind,
once I'm instantiating an actual branch object, it should be fully
defined what branch I am getting, it shouldn't go and search.
If I am calling a function named "find_branch" then I obviously realize
that I am searching for the root.

Also, I wouldn't change the name of the parameter from "base" to "f".
Base has quite a bit more meaning.

Is there any reason that RemoteBranch uses "self.baseurl" instead of
"self.base"? It means that when you instantiate a RemoteBranch object,
you have to realize what type it is (or look for both).
If baseurl is important, maybe it could define both.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 253 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20050626/6338cfd3/attachment.pgp