[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