Launchpad itself

Code review comment for lp://staging/~jtv/launchpad/translations-person-3.0-mechanical

translations-person-3.0-mechanical
Merge into devel

Revision history for this message

Jeroen T. Vermeulen (jtv) wrote on 2009-09-01:

These are mostly basic, semi-mechanical migrations of the person-related
Translations pages to the 3.0 templates.

Most of these are very simple: there are very few actions on these pages
and the 2.0 changes left them without any portlets that might be missed.
Two navigation menus are involved (for Person and POFile), and some
groundwork having been laid already, reusing those as related-pages
menus was entirely painless in both cases.

== Details in diff order ==

Some page titles were moved into views. Saved some trouble with the
filtered-translations view, which in a way has two contexts: a Person
whose translations you're viewing and the POFile that those translations
are in.

Since we were still painstakingly composing links to translation groups,
I bit the bullet and created a formatter for them. Used on the personal
Translations page and the translation groups index. Surprisingly easy;
no idea why we never did this before.

In the relicensing view, at the bottom of browser/person.py, I removed
the sentence telling a user after declining to BSD-license their
translations that their existing translations will be removed. We were
going to do that during the BSD licensing migration, and we did. We're
not planning to do that any more now that everyone's had ample time to
choose.

In test_translationlinksaggregator.py I fixed a test that I stupidly
left brittle, depending on the order of a list that isn't actually in a
guaranteed order. Others have run into this problem with the test as
well, but this instance was still present in trunk.

In TranslationImportQueueView, the initialize method can now safely
re-use its parent method instead of duplicating its work. That's why
_initial_values is no longer set there: the parent does it.

In TranslationRelicensingAgreementOptions, I shortened the option labels
after discussion with noodles, mpt, and danilo. It all started with the
labels having a "nowrap" style and so happily stretching out of a narrow
browser window.

Almost all the pages I edited got a "related pages" section at the
bottom in return for their old navigation menu. The exception is the
relicensing page, which reflects a setting and so got a Cancel link
instead. It leads back to the person's main translations page.

On the relicensing page I also took the opportunity to update more of
the wording. Some of it struck me as a bit awkward; the changes are not
very big.

Then there's pofile-filter.pt. It needed a bit more work: there was
popup help that explained how different CSS styles on the page mark
different kinds of messages. That is inherently not self-evident no
matter how much else you know about the application, and easily
forgotten unless you use that page very frequently (which most people
who use it probably don't). On the other hand there really wasn't
enough extra information to justify a separate "page." So I turned it
into a quick inline sentence.

The translation-import-queue-macros.pt template is not a page, so it
didn't strictly need updating to 3.0. But it missed some chances for an
easy fmt:link so I fixed that.

== Tests ==

I mostly just kept running the Translations stories and browser tests:
{{{
./bin/test -vv -t 'translations.*stories' -t 'translations.*browser'
}}}

== Demo and Q/A ==

Go to the home page of someone who is active in translations (the more
active, the more you will see—especially if you are that person) and
select the Translations tab.

Cycle through the list of related pages; all those should be converted.
From the Translations licensing page you can get back to the rest of the
set using the Cancel link at the bottom.

Also, select a translation that the person has done. This will bring
you to the pofile-filter page. It looks radically different, mostly
because of the removed portlets. Those were entirely redundant with the
POFile's description page as found in the related pages. You'll also
find that the page now links back to the person.

== Lint ==

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/pagetitles.py
  lib/canonical/launchpad/webapp/configure.zcml
  lib/canonical/launchpad/webapp/tales.py
  lib/lp/translations/browser/hastranslationimports.py
  lib/lp/translations/browser/person.py
  lib/lp/translations/browser/pofile.py
  lib/lp/translations/browser/translationimportqueue.py
  lib/lp/translations/browser/tests/test_translationlinksaggregator.py
  lib/lp/translations/interfaces/translationrelicensingagreement.py
  lib/lp/translations/stories/standalone/xx-licensing.txt
  lib/lp/translations/stories/standalone/xx-person-translations.txt
  lib/lp/translations/templates/hastranslationimports-index.pt
  lib/lp/translations/templates/person-translations-relicensing.pt
  lib/lp/translations/templates/person-translations.pt
  lib/lp/translations/templates/pofile-filter.pt
  lib/lp/translations/templates/translation-import-queue-macros.pt
  lib/lp/translations/templates/translationgroups-index.pt
  lib/lp/translations/templates/translationimportqueue-index.pt

=== Pylint notices ===

lib/canonical/launchpad/webapp/tales.py
20: [F0401] Unable to import 'lazr.enum' (No module named enum)

lib/lp/translations/interfaces/translationrelicensingagreement.py
6: [F0401] Unable to import 'lazr.enum' (No module named enum)

Don't know what it's complaining about, to be honest. It doesn't seem
to be anything to do with my changes, and I'd rather not mess with them
in case it's a transient problem with local setup.

Jeroen

= Bug 422462 =

These are mostly basic, semi-mechanical migrations of the person-related
Translations pages to the 3.0 templates.

== Details in diff order ==

Some page titles were moved into views.  Saved some trouble with the
filtered-translations view, which in a way has two contexts: a Person
whose translations you're viewing and the POFile that those translations
are in.

Since we were still painstakingly composing links to translation groups,
I bit the bullet and created a formatter for them.  Used on the personal
Translations page and the translation groups index.  Surprisingly easy;
no idea why we never did this before.

In the relicensing view, at the bottom of browser/person.py, I removed
the sentence telling a user after declining to BSD-license their
translations that their existing translations will be removed.  We were
going to do that during the BSD licensing migration, and we did.  We're
not planning to do that any more now that everyone's had ample time to
choose.

In test_translationlinksaggregator.py I fixed a test that I stupidly
left brittle, depending on the order of a list that isn't actually in a
guaranteed order.  Others have run into this problem with the test as
well, but this instance was still present in trunk.

In TranslationImportQueueView, the initialize method can now safely
re-use its parent method instead of duplicating its work.  That's why
_initial_values is no longer set there: the parent does it.

In TranslationRelicensingAgreementOptions, I shortened the option labels
after discussion with noodles, mpt, and danilo.  It all started with the
labels having a "nowrap" style and so happily stretching out of a narrow
browser window.

Almost all the pages I edited got a "related pages" section at the
bottom in return for their old navigation menu.  The exception is the
relicensing page, which reflects a setting and so got a Cancel link
instead.  It leads back to the person's main translations page.

On the relicensing page I also took the opportunity to update more of
the wording.  Some of it struck me as a bit awkward; the changes are not
very big.

Then there's pofile-filter.pt.  It needed a bit more work: there was
popup help that explained how different CSS styles on the page mark
different kinds of messages.  That is inherently not self-evident no
matter how much else you know about the application, and easily
forgotten unless you use that page very frequently (which most people
who use it probably don't).  On the other hand there really wasn't
enough extra information to justify a separate "page."  So I turned it
into a quick inline sentence.

The translation-import-queue-macros.pt template is not a page, so it
didn't strictly need updating to 3.0.  But it missed some chances for an
easy fmt:link so I fixed that.

== Tests ==

I mostly just kept running the Translations stories and browser tests:
{{{
./bin/test -vv -t 'translations.*stories' -t 'translations.*browser'
}}}

== Demo and Q/A ==

Go to the home page of someone who is active in translations (the more
active, the more you will see—especially if you are that person) and
select the Translations tab.

Cycle through the list of related pages; all those should be converted.
From the Translations licensing page you can get back to the rest of the
set using the Cancel link at the bottom.

Also, select a translation that the person has done.  This will bring
you to the pofile-filter page.  It looks radically different, mostly
because of the removed portlets.  Those were entirely redundant with the
POFile's description page as found in the related pages.  You'll also
find that the page now links back to the person.

== Lint ==

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

=== Pylint notices ===

lib/canonical/launchpad/webapp/tales.py
    20: [F0401] Unable to import 'lazr.enum' (No module named enum)

lib/lp/translations/interfaces/translationrelicensingagreement.py
    6: [F0401] Unable to import 'lazr.enum' (No module named enum)

Don't know what it's complaining about, to be honest.  It doesn't seem
to be anything to do with my changes, and I'd rather not mess with them
in case it's a transient problem with local setup.

Jeroen

« Back to merge proposal