[MERGE] working tree cleanups

[Robert Collins]

On Sat, 2006-07-15 at 10:15 -0500, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > On Fri, 2006-07-14 at 09:19 -0500, John Arbash Meinel wrote:
> > 
> >>> === modified file 'bzrlib/workingtree.py'
> >>> --- bzrlib/workingtree.py	2006-07-03 18:27:35 +0000
> >>> +++ bzrlib/workingtree.py	2006-07-13 10:45:47 +0000
> >>> @@ -53,6 +53,8 @@
> >>>  
> >>>  from bzrlib import bzrdir, errors, osutils, urlutils
> >>>  from bzrlib.atomicfile import AtomicFile
> >>> +import bzrlib.branch
> >>> +import bzrlib.bzrdir as bzrdir
> >> ^^^ we already import bzrdir
> >>
> >> from bzrlib import bzrdir
> >>
> >> and I think that is the preferred form now. I know it will work much
> >> better with demandload. And I believe pyflakes complains less.
> > 
> > AIUI from bzrlib import bzrdir will not work unless bzrdir is already
> > imported by the codebase - bzrdir is an attribute of bzrlib only after
> > the import of the children is complete. I may be wrong here - its
> > possibly worth a quick test. Anyway, I'm happy to remove the 'from
> > bzrlib import bzrdir' instance, which is not in alphabetical order :).
> > 
> 
> No, I already tested that when we were proposing to change it. It does
> the right thing *unless* you have a local variable 'bzrdir' inside the
> module.
> 
> % mkdir foo
> % mkdir foo/bar
> % echo "print 'import foo'" > foo/__init__.py
> % echo "print 'import bar'" > foo/bar/__init__.py
> 
> % python -c 'from foo import bar; print bar'
> import foo
> import bar
> <module 'foo.bar' from 'foo/bar/__init__.pyc'>
> % echo "bar = 'hello'" >> foo/__init__.py
> % python -c 'from foo import bar; print bar'
> import foo
> hello
> 
> I looked into this quite a bit. and 'from bzrlib import bzrdir' works,
> and works better with demandload. I really prefer it if we switch.
> 
> I'm not sure what you mean by 'it isn't in alphabetical order'. I would
> say a plain 'from bzrlib' comes before any other imports.
> 
> Maybe you would prefer:
> 
> from bzrlib import (
> 		    bzrdir,
> 		    errors,
> 		    osutils,
> 		    urlutils,
> 		   )
> 
> But I do think we want a single 'from bzrlib' rather than lots of:
> 
> from bzrlib import bzrdir
> from bzrlib import errors
> from bzrlib import ...

Sure. Given you've tested the import works correctly, I'll change to
suit. I do find it *extremely* amusing that my original changes to
builtins.py when I added bzrdir, using just 'bzrdir' to refer to the
module, which was rejected, are now considered best practice :)

> 
> > It sure was returning something. This is the old code:
> >             raise NotImplementedError
> >         if not isinstance(a_bzrdir.transport, LocalTransport):
> >             raise errors.NotLocalUrl(a_bzrdir.transport.base)
> >         control_files = self._open_control_files(a_bzrdir)
> >         return WorkingTree3(a_bzrdir.root_transport.local_abspath('.'),
> >                             _internal=True,
> >                             _format=self,
> > 
> > 
> > I'll hold of merging this till we have a clear consensus on the import
> > thing.
> > 
> > -Rob
> 
> Sure, I was just misreading the diff. I'm okay with the change. Just the
> import thing now.


No problem, merging now.

Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060717/07c9ffaf/attachment.pgp