[merge] more LockableFiles deprecations and misc cleanups
Ian Clatworthy
ian.clatworthy at canonical.com
Wed May 21 05:13:38 BST 2008
Martin Pool wrote:
> I can see one nit looking over this but I'd like to send it
> provisional review anyhow:
>
> - # self._transport used to point to the directory containing the
> - # control directory, but was not used - now it's just the transport
> - # for the branch control files. mbp 20070212
> self._base = self.bzrdir.transport.clone('..').base
> + # self.base = self.bzrdir.root_transport.base
>
> The code that is still there, setting _base, seems a bit wonky, but I
> didn't finish changing it. What this wants is to get the user-visible name
> of the branch (the directory containing .bzr) and it should ask the bzrdir
> for this.
Thanks for explicitly mentioning this is your covering email. I was wondering
why you left the commented code in there.
> The changes to use transport.put_bytes() etc are somewhat overlapping
> with my other patch to get the permissions from the BzrDir rather than
> LockableFiles, but I thought they would be easier to review separately.
Yes. There are lots of places where mode=xxx.control_files.... is used
where I was expecting something different having just completed reviewing
your other patch. :-) Were you planning to make those while merging?
Given the number, perhaps a separate patch covering those is worth it.
bb:tweak
> .. contents::
>
> + API BREAKS:
Remember to move these inside IN DEVELOPMENT when merging please.
> @@ -1802,10 +1802,10 @@
> :param location: URL to the target branch
> """
> if location:
> - self.control_files.put_utf8('bound', location+'\n')
> + self._transport.put_bytes('bound', location+'\n')
In branch.py here, do you need to pass the mode to put_bytes?
> def put_format(self, dirname, format):
> - self.bzrdir._control_files.put_utf8('%s/format' % dirname, format.get_format_string())
> + self.bzrdir.transport.put_bytes('%s/format' % dirname,
> + format.get_format_string(),
> + self.file_mode)
In bzrlib.py here, is self.file_mode guaranteed to be set before
this method is called?
Otherwise, all looks good.
Ian C.
More information about the bazaar
mailing list