Merge lp:~jtv/launchpad/recife-approveSuggestion into lp:~launchpad/launchpad/recife
Status: | Merged |
---|---|
Approved by: | Graham Binns |
Approved revision: | no longer in the source branch. |
Merged at revision: | 9157 |
Proposed branch: | lp:~jtv/launchpad/recife-approveSuggestion |
Merge into: | lp:~launchpad/launchpad/recife |
Diff against target: | 0 lines |
To merge this branch: | bzr merge lp:~jtv/launchpad/recife-approveSuggestion |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Graham Binns (community) | code | Approve | |
Review via email: mp+32848@code.launchpad.net |
Commit message
TranslationMess
Description of the change
= Approving Suggestions =
For the Recife feature branch. I learned one big advantage of TDD while working on this branch: writing the tests brought the diff close to the review limit, and implementation was almost an afterthought. (I even had a working filthy-hack shortcut implementation for a while that passed tests). Watching the tests grow before you consider implementation is a great way to keep yourself under the limit.
Here I implement a new method: TranslationMess
The method on POTMsgSet re-uses the bulk of setCurrentTrans
The first thing you'll notice in the diff, however, is a new factory method: makeCurrentTran
The diff gets awkward in lib/lp/
In what is now _setTranslation (previously setCurrentTrans
To test:
{{{
./bin/test -vvc -m lp.testing.
./bin/test -vvc -m lp.translations
./bin/test -vvc -m lp.translations
}}}
I eliminated all lint, save two identical cases in IPOTemplate where the linter reads comments as blank lines and so claims that there are 2 blank lines between consecutive methods. I say it's wrong.
Jeroen
Hi Jeroen,
I'm happy with this branch, but you need to add some comments or docstrings to the new tests in test_translatio nmessage. py; I shouldn't have to read a unit test to find out what it tests ;).
Other than that this is fine, so I'll mark it approved pending the comments.