[merge] avoid RemoteLockableFiles special handling of branch.conf (and query: purpose of Branch.get_config_file rpc)

Martin Pool mbp at canonical.com
Wed May 7 00:51:51 BST 2008


On Tue, May 6, 2008 at 10:54 PM, John Arbash Meinel
<john at arbash-meinel.com> wrote:

>  I don't understand why Branch.get_config() couldn't be updated for
>  RemoteBranch.get_config() so that you can use the RPC.
>
>  I guess because TreeConfig peeks and does:
>  ~        transport = branch.control_files._transport
>  ~        self._config = TransportConfig(transport, 'branch.conf')
>  ~        self.branch = branch
>
>  which is buried fairly deep underneath "BranchConfig()".
>
>  I certainly would rather see RemoteBranch.get_config() returning a
>  RemoteBranchConfig object, or a BranchConfig object that is using a better
> api than a raw transport + filename.

I agree that would be the right way to take it, but we can clean this
up for now while still staying based on the Transport.

>  You patch doesn't quite match my bzr.dev, since there is no
> @deprecated_method()
>  for 'get()'.

Yes, it was based on top of my branch deprecating that, and I realized
it was quite separate.  I thought I'd get opinions before making a new
branch.

>  Also, as you mentioned, we are calling directly to Transport.get() not
>  LockableFiles.get() so probably this function is not being called. (I
> assume
>  this was part of the earlier refactoring to stop going through
> LockableFiles and
>  go more directly to the Transport.)

In fact it is never hit when running the tests.

>  So +1 on removing the RemoteLockableFiles.get() method (though a
> deprecation
>  *might* be better).

I'm going to at least gut it, removing this special case.  My other
branch will deprecate LockableFiles.get() in general.

Thanks very much for the review.
-- 
Martin <http://launchpad.net/~mbp/>



More information about the bazaar mailing list