Code review comment for lp:~james-w/launchpad/fix-publishing-getbyid

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Hi James,

Thanks for the cleanups.

Was changing
lib/lp/soyuz/scripts/tests/upload_test_files/drdsl_1.2.0.orig.tar.gz intentional ?

The class definition of PublishingSetTests needs to be preceded by two empty lines.

There seems to be quite a bit of duplication in the various methods in PublishingSetTests, perhaps some of that code could be moved into a setUp() method.

I'm bothered by the size of the test factory and its lack of tests. Seeing it extended further worries me a bit, though I don't have any easy alternatives.

review: Needs Information (code)

« Back to merge proposal