Code review comment for lp:~michael.nelson/launchpad/429318-copy-archives-timeout

 merge approve
 review approve

On Thursday 17 September 2009 08:18:08 Michael Nelson wrote:
> Yes - I was only thinking of PPAs when I wrote it. So I'm all for the
> new version, I just wasn't sure if selecting SourcePackageReleases was
> the best way.

Well I was thinking of the previous use of Python sets rather than distinct
SQL clauses :)

> Yes, I wonder whether it would even be worthwhile splitting the content
> class up using mixins to match the documentation. Also think it would be
> great to improve the documentation by moving lots of the unit tests
> there to actual python unittests, leaving the documentation much more
> readable. But I think that would take planning and time?

Yes, it's certainly something for a rainy day.

For me, doctests should contain basic instructions on how to use the content
class and its methods. Testing corner cases belong in a unit test, but I
don't think your new test is that.

Our problem is that Soyuz depends heavily on setting up a lot of data, so
doctests tend to be full of calls to the STP and factory, which muddies the
story, so I can see why you made it a unit test.

Ho hum.

> For the moment, I've added a brief section in archive.txt with a
> reference to the unit test. See what you think.

It's fine, thanks.

Land it!