Comment 37 for bug 2627

Revision history for this message
Martin Pool (mbp) wrote : Re: [Bug 2627] Re: Non-leading whitespace mangled in bug description and comments

Paul, thanks so much for tackling this, ascii art connoisseurs
everywhere will rejoice when it's fixed.

I think to make sure if this is actually what you want you are going
to need to run it. It's not trivial to get lp running but it's not
all that bad, and people in #launchpad-dev can help.

As far as the actual patch
<http://bazaar.launchpad.net/~sladen/launchpad/compress-whitespace-lp2627/revision/14130>

 * you are turning this on everywhere that uses text_to_html, not just
bug descriptions, and that's probably too much

 * there is still the issue I raised above that pre blocks never(?)
wrap. (I guess you can change that in css but you would have to do
that.)

 * is doing <pre> one line at a time a good idea?

The addition of the feature flag check is basically ok though:

 * the name implies it's only for bugs but as I said this will change
just about all variable text in lp

 * you need to document the flag in flags.py

> In addition to adding the flag description, the documentation at:
> https://dev.launchpad.net/FeatureFlags
>states that one must add a new scope class in:
> lib/lp/services/features/scopes.py
> but this file appears to be mostly empty.

 * it's not empty
http://bazaar.launchpad.net/~sladen/launchpad/compress-whitespace-lp2627/view/head:/lib/lp/services/features/scopes.py

 * you only need to do this if you want to add a new scope, which is
not necessary here.

> Is all that is needed to query a FeatureFlag the single 'getFeatureFlag()' call?

for checking an existing flag yes, but here also
https://dev.launchpad.net/FeatureFlags#Adding_and_documenting_a_new_feature_flag

there are also probably some tests to update, but that can wait until
you're happy with the way it looks.

cheers