Merge lp:~jelmer/launchpad/635591-sync-source-unicode into lp:launchpad
Proposed by
Jelmer Vernooij
Status: | Merged |
---|---|
Approved by: | Jelmer Vernooij |
Approved revision: | no longer in the source branch. |
Merge reported by: | Jelmer Vernooij |
Merged at revision: | not available |
Proposed branch: | lp:~jelmer/launchpad/635591-sync-source-unicode |
Merge into: | lp:launchpad |
Diff against target: |
63 lines (+28/-1) 2 files modified
lib/lp/soyuz/scripts/ftpmaster.py (+1/-1) lib/lp/soyuz/scripts/tests/test_sync_source.py (+27/-0) |
To merge this branch: | bzr merge lp:~jelmer/launchpad/635591-sync-source-unicode |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | code | Approve | |
Review via email: mp+35637@code.launchpad.net |
Commit message
Fix handling of non-UTF8 changelog entries in sync-source.
Description of the change
This branch fixes the ability to sync sources that contain a changelog entry with non-ASCII UTF8 characters.
I've moved generate_changes() so that it would be possible to write unit tests for it.
tests: ./bin/test lp.soyuz.
To post a comment you must log in.
Nice branch :)
A few comments, but they're all minor. +1
[1]
There's some trailing whitespace in ftpmaster.py. There are some other
lint warnings (especially from pep8) but I don't know if any of them
are related to your changes. If you have time, consider fixing them,
but don't worry too much.
[2]
+ # Strip trailing newline
+ return changes
I think that comment can go.
[3]
+ self.assertFals e("Description" in changes) e("Closes" in changes) e("Launchpad- bugs-fixed" in changes) e("Launchpad- bugs-fixed" in changes)
+ self.assertFals
+ self.assertFals
...
+ self.assertFals
If you were to switch the base class from unittest.TestCase to
lp.testing.TestCase (or testtools.TestCase, its parent) then you could
use assertNotIn() in these places.
[4]
+ self.assertTrue( find("Updated French translation by J\xc3\xa9lmer."))
+ contents.
I think this would be clearer as:
Or, better:
[5]
filehandle = open(output_ filename, 'w') write(changes+ '\n') dump(filehandle , encoding="utf-8")
- # XXX cprov 2007-07-03: The Soyuz .changes parser requires the extra '\n'
- filehandle.
- filehandle.close()
+ try:
+ changes.
+ finally:
+ filehandle.close()
Consider using "with" here.