[MERGE][RFC] switch for heavyweight checkouts
Ian Clatworthy
ian.clatworthy at internode.on.net
Fri Dec 7 06:34:31 GMT 2007
John Arbash Meinel wrote:
> I'm happy to see switch for heavy checkouts implemented (as then I might
> actually start using it, we shall see, but then again, maybe I should keep
> paying the build_tree price so that I'm motivated to fix it).
Thanks for the review.
> Having thought about it a bit, I think I agree that to switch from a missing
> branch you should supply '--force'.
>
> There is a subtle but important difference from "the branch moved, point at
> this new location" versus "I'm done with this, and want to work on something
> else." By silently ignoring a missing branch, the "I'm done with this" may not
> really mean that.
Done.
> ^- I'm very happy to see set_reference be a method/class method rather than
> having it in switch.py.
>
> I didn't see any direct tests of it, though. A simple smoke test is enough
> (just so that if someone changes the api, deletes it, etc, it tells us).
Done.
> + if not force and _any_local_commits(b, bound_branch):
>
> ^- I would tend to not pass the url, and just have the helper use
> "b.get_master_branch()" instead.
Done.
> Also, should we be passing around possible transports?
Done. I've submitted the tweaks to pqm but I'd appreciate it if you
could double-check what I did here. I think my changes are safe but
perhaps I missed some API calls which would benefit from this.
> + remote_graph = other_branch.repository.get_revision_graph(
> + other_last_rev)
> + if last_rev not in remote_graph:
> + return True
>
> ^- Using get_revision_graph() is slow. You should instead do:
>
> graph = this_branch.repository.get_graph(other_branch.repository)
> if graph.is_ancestor(last_rev, other_last_rev):
> return True
>
> This can look at just the tip of the graph. And using "this.get_graph(other)"
> will allow us to use a StackedParentsProvider that allows us to pull revisions
> we have from the local side, rather than downloading all of the revision graph
> from the remote side.
Sweet. FWIW, I copied that code from bind() in branch.py. :-( It's a
little more complex but perhaps it could benefit in much the same way?
Ian C.
More information about the bazaar
mailing list