[MERGE] code cleanups for bzrdir.py

[Martin Pool]

Martin Pool has voted tweak.
Status is now: Conditionally approved
Comment:
@@ -234,12 +236,12 @@
          t.ensure_base()
          if format is None:
              format = BzrDirFormat.get_default_format()
-        return format.initialize(base, possible_transports)
+        return format.initialize_on_transport(t)

(comment) This is 'leaking' the possible_transports list.  Of course 
they will be gc'd, but it does mean that initialize_on_transport and 
things that will call will no longer have the chance to reuse them. 
That's probably not a problem though because anything related can still 
be reconstructed from t.  But in general anything that gets a 
possible_transports should pass it on (yes?)

@@ -569,12 +578,13 @@
              note('%s is%s redirected to %s',
                   transport.base, e.permanently, target)
              # Let's try with a new transport
-            qualified_target = e.get_target_url()[:-len(relpath)]
              # FIXME: If 'transport' has a qualifier, this should
              # be applied again to the new transport *iff* the
              # schemes used are the same. It's a bit tricky to
              # verify, so I'll punt for now
              # -- vila20070212
+            #qualified_target = e.get_target_url()[:-len(relpath)]
+            #return get_transport(qualified_target)
              return get_transport(target)

I'd rather this were replaced with a comment about what it's trying to 
do.



+class RetireFailed(BzrDirError):
+
+    _fmt = "Failed to retire %(display_url)s."
+
+

I'm not sure this error will be very meaningful to users, or that any 
callers would specifically want to catch it.  Also it's just hiding the 
real underlying reason.  I think you should instead just re-raise the 
original error if we've got tired of retrying.


For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C46EF7C4F.8020404%40internode.on.net%3E