[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