[MERGE] code cleanups for bzrdir.py

[Ian Clatworthy]

Updated bundle attached.

Martin Pool wrote:
> Martin Pool has voted tweak.

> (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?)

As discussed on IRC, the general rule is right but, I believe, not
meaningful in this case. (We already have decided the transport by the
time initialize_on_transport is called.)

> +            #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.

Comment updated with suggested code. Code snippet also added to #122258
as vila has suggested that bug is strongly related.

> +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.

Thanks for explaining this further on IRC. I've changed the code as
suggested and added a test to confirm it works.

Ian C.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzrdir-code-cleanups2.patch
Type: text/x-patch
Size: 19473 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070921/315e5b1b/attachment.bin