[MERGE REVIEW] create_checkout_convenience

Martin Pool mbp at canonical.com
Mon Aug 7 06:00:35 BST 2006


On  4 Aug 2006, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
> I want to get nested trees in, but it's a big chunk to digest.  There
> are a few bits I can break out, and this is one.
> 
> It introduces create_checkout_convenience, which was something I needed
> for the nested-trees test cases.  No reason it can't go in first, right?

I like the concept and that you removed this from the builtin command.

However I think the proliferation of staticmethods for such things is a
bad trend and I'd rather we don't add another.  Somehow they don't seem
to work well in Python, partly because there's just a single namespace
for instance variables, instance methods, and class methods.

When talking to Robert the other day I proposed this rule: static/class
methods should be confined to the "named constructor" pattern.

> === modified file bzrlib/bzrdir.py
> --- bzrlib/bzrdir.py
> +++ bzrlib/bzrdir.py
> @@ -284,7 +284,33 @@
>              except errors.NotLocalUrl:
>                  pass
>          return result
> +
> +    @staticmethod
> +    def create_checkout_convenience(to_location, source, revision_id=None,
> +                                    lightweight=False):
> +        """Create a checkout of a branch.
>          
> +        :param to_location: The url to produce the checkout at
> +        :param source: The branch to produce the checkout from
> +        :param revision_id: The revision to check out
> +        :param lighweight: If True, produce a lightweight checkout, othewise
> +        produce a bound branch (heavyweight checkout)
> +        :return: The tree of the created checkout
> +        """
> +        if lightweight:
> +            checkout = BzrDirMetaFormat1().initialize(to_location)
> +            bzrlib.branch.BranchReferenceFormat().initialize(checkout, source)
> +        else:
> +            checkout_branch =  BzrDir.create_branch_convenience(
> +                to_location, force_new_tree=False)
> +            checkout = checkout_branch.bzrdir
> +            checkout_branch.bind(source)
> +            if revision_id is not None:
> +                rh = checkout_branch.revision_history()
> +                checkout_branch.set_revision_history(rh[:rh.index(revision_id) + 1])
> +        return checkout.create_workingtree(revision_id)
> +
> +    
>      @staticmethod
>      def create_repository(base, shared=False):
>          """Create a new BzrDir and Repository at the url 'base'.

Could these perhaps instead be, say, an instance method of source?  You
must already have one before starting, and it seems reasonable to say
"please check yourself out".o

And also the 'lightweight' parameter basically runs two different
methods, which suggests we should actually have two.  So how abut

  source.create_lightweight_checkout(to_location, ...)

  source.create_bound_branch(to_location, ...)

I realize the others already work this way but if this can be agreed for
this new one we can fix the others.

-- 
Martin




More information about the bazaar mailing list