Code review comment for lp:~jelmer/bzr/colo-urls

Revision history for this message
Martin Pool (mbp) wrote :

I'm still a bit concerned that this is passing the branch name around so many different places, rather than currying that into an object. I guess the problem here is that the places you're updating here are "places a branch could be" not necessarily an existing open branch. Aside from that it looks pretty good, and perhaps it's easier to bring it in and then change it later.

- this_branch._format.set_reference(this_branch.bzrdir, other_branch)
+ this_branch._format.set_reference(this_branch.bzrdir, None,
+ other_branch)

Inserting parameters into the middle is a somewhat noxious way to change the api in Python. It's ok to have an api bump but perhaps we should add to the end?

« Back to merge proposal