[MERGE][Bug #273993] Add --no-tree option to `bzr branch`

[Daniel Watkins]

Hi John,

Thanks for the speedy review.

On Sat, 2009-02-07 at 12:01 -0600, John Arbash Meinel wrote:
>      def run(self, from_location, to_location=None, revision=None,
> - -            hardlink=False, stacked=False, standalone=False):
> +            hardlink=False, no_tree=False, stacked=False,
> standalone=False):
>          from bzrlib.tag import _merge_tags_if_possible
> 
> 
> ^- minor thing, we generally always add new parameters at the end. I'm
> curious what made you decide to put it here (alphabetically sorted?)
I think it was a combination of alphabetically sorted, and trying to
preserve the original patch as far as possible.  Neither of which is
really a good reason.  This is fixed in the attached.

> +        if (isinstance(target_transport, local.LocalTransport) and not
> no_tree
> +            and (result_repo is None or result_repo.make_working_trees())):
>              wt =
> result.create_workingtree(accelerator_tree=accelerator_tree,
>                  hardlink=hardlink)
>              wt.lock_write()
> 
> ^- I would just start with:
> 
> if not no_tree ...
> 
> I wonder if it wouldn't be better to change the parameter to:
> 
> ..., create_tree_if_local=True)
> 
> Since it is usually better to use a positive statement, than "not no_tree".

I agree.  Fixed.

> ...
> 
> - -        # we always want a working tree
> - -        WorkingTreeFormat2().initialize(result,
> - -
> accelerator_tree=accelerator_tree,
> - -                                        hardlink=hardlink)
> +
> +        if not no_tree:
> +            WorkingTreeFormat2().initialize(result,
> +
> accelerator_tree=accelerator_tree,
> +                                            hardlink=hardlink)
> 
> ^- This is wrong. Formats that use WorkingTreeFormat2 *have* to have a
> working tree. (This is really old stuff; we shouldn't change it)

I wondered if this was the case as I changed it, but couldn't come to
any good conclusion by myself.  Fixed.


Dan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 273993.2.patch
Type: text/x-patch
Size: 13330 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090208/6f1379e0/attachment.bin