Code review comment for lp:~barry/launchpad/394133-urlwrap

Revision history for this message
Barry Warsaw (barry) wrote :

> Two remarks about this branch:
>
> - Your changes avoid a broken URL in "moderation approval messages".
> But I am wondering, if this should perhaps be applied to other
> messages as well. After all, wrapped URLs for example in bug
> comments are bad too. (OK, in this case you can visit the related
> web page to see the "good" URL)

I think we can handle this on a case-by-case basis. If we get additional bug reports we can implement a no-wrap filter function at that time.

> - Lines are only wrapped iff the paragraph starts with "http:" or
> "https:", so it won't work when the paragraph contains a few
> introductory words before the URL.

This is true, however if we run across such a situation, we can change the no-wrap filter function to suite the particular situation. The one implemented in this fix is only meant to cover the new-held-message email message.

>
> OTOH, you write that the wrapper in Python 2.6 will no longer need
> this workaround, so I think this highly specialized fix is fine.
>

Thanks! Yes, it will be better to use Python 2.6's facilities when we land on that version. Thanks for the review!

« Back to merge proposal