Merge lp:~jtv/launchpad/bug-702832 into lp:launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Curtis Hovey | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 12221 | ||||
Proposed branch: | lp:~jtv/launchpad/bug-702832 | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
94 lines (+29/-12) 3 files modified
lib/lp/translations/model/potmsgset.py (+3/-4) lib/lp/translations/tests/test_potmsgset.py (+0/-7) lib/lp/translations/tests/test_translationmessage.py (+26/-1) |
||||
To merge this branch: | bzr merge lp:~jtv/launchpad/bug-702832 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+46261@code.launchpad.net |
Commit message
[r=sinzui]
Description of the change
= Bug 702832 =
Fixes a relatively rare oops in the "Recife" Translations code that was written over the past year and finally rolled out last cycle. A certain piece of data used to be structured as either a list of a message's translations for each of the language's plural forms, or a dict mapping the plural forms to their corresponding translations. Duck typing still made things work pretty well when a list was passed. One call site still passed a list, and the docstring still, mistakenly, asked for one.
The incorrect type only caused trouble when the data was then passed on to yet another method that did expect a dict. For invocations from the call site that made this mistake, that last method was only called in a rare scenario. That's how this managed to escape both unit tests (which passed the right type) and integration tests (which didn't tickle this particular, rare combination of calls).
Run this test to verify that the specific bug is gone:
{{{
./bin/test -vvc lp.translations
}}}
I also removed an assertion that seems a bit silly given later changes: it checks that a required parameter is not None, but it's pretty much always going to be either a constant or a not-null database value. That also eliminated a now-pointless test.
For Q/A, we should see that approval of existing suggestions and translations still works.
No lint,
Jeroen
This look good to Land. Thank you for removing the cruft.