Merge lp://staging/~sinzui/launchpad/poimport-typeerror into lp://staging/launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Curtis Hovey | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 16261 | ||||
Proposed branch: | lp://staging/~sinzui/launchpad/poimport-typeerror | ||||
Merge into: | lp://staging/launchpad | ||||
Diff against target: |
141 lines (+82/-22) 2 files modified
lib/lp/translations/model/pofile.py (+24/-21) lib/lp/translations/tests/test_pofile.py (+58/-1) |
||||
To merge this branch: | bzr merge lp://staging/~sinzui/launchpad/poimport-typeerror | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | code | Approve | |
Review via email: mp+133713@code.staging.launchpad.net |
Commit message
Ensure the error data is sane enough to send an error email.
Description of the change
The TypeError happens when the script is trying to log an error. All the
data is sanitised except for the call to
potmsgset.
None if the TTI is incomplete. We just want to ensure the data is sane
enough so that we can log the error.
-------
RULES
Pre-
* Extract the logic that makes the error email to a separate testable
method.
* Add a test for the current behaviour.
* Update the method to fallback to -1 if the sequence is None
* Revise the loop to avoid needless string concatenation of errordetails
QA
* Untestable, the case happens when the object is in an invalid state.
LINT
lib/
lib/
LoC
I have a credit of 4000 lines.
TEST
./bin/test -vvc -t prepare_pomessage lp.translations
IMPLEMENTATION
Extracted the logic to a method and added a test. Added a rule to use -1
of the sequence is None.
lib/
lib/
Hi Curtis,
Typo: enail
When there is no sequence will the users understand '-1'? Would another string like "Invalid sequence" be better?
Overall it looks good.