Code review comment for lp:~jelmer/launchpad/635591-sync-source-unicode

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.assertFalse("Description" in changes)
+ self.assertFalse("Closes" in changes)
+ self.assertFalse("Launchpad-bugs-fixed" in changes)
...
+ self.assertFalse("Launchpad-bugs-fixed" in changes)

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(
+ contents.find("Updated French translation by J\xc3\xa9lmer."))

I think this would be clearer as:

        self.assertTrue(
            "Updated French translation by J\xc3\xa9lmer." in contents)

Or, better:

        self.assertIn(
            "Updated French translation by J\xc3\xa9lmer.", contents)

[5]

     filehandle = open(output_filename, 'w')
- # XXX cprov 2007-07-03: The Soyuz .changes parser requires the extra '\n'
- filehandle.write(changes+'\n')
- filehandle.close()
+ try:
+ changes.dump(filehandle, encoding="utf-8")
+ finally:
+ filehandle.close()

Consider using "with" here.