Code review comment for lp:~wgrant/launchpad/distroseries-source-format-selection-part1

Hi William, thanks for making this change. There's some much-needed refactoring and clearing up in nascentupload.

The basic direction is good, I just have a few comments to make for things that need fixing. Gavin will do a more thorough review as we agreed on IRC.

General questions:

 * I can't tell from the diff but are the db permissions added for the new table correctly tested in the tests? This usually just means ensuring that the test runs as the correct db user.
 * The XXX about the orphaned files is interesting. I don't think we deal with this right now do we? I'm trying to think if it would be bad to reject the upload if we detect orphan files, and I can't think of one. What's your opinion?

Things that need fixing:

 * The upload format check is not tested. (The "%s: format '%s' is not permitted in %s." one)

 * IDistroSeries methods permitSourcePackageFormat() and isSourcePackageFormatPermitted() should be in a utility, e.g. SourcePackageFormatSet. This ensures that in a zopeful environment they are security wrapped. Although the upload processor is zopeless, we might need to do that one day. Something like
   * SourcePackageFormatSet.add()
   * SourcePackageFormatSet.getByFormat()
   would fit with our current style. You can keep the isSourcePackageFormatPermitted() and make it call the utility method as a convenience, but the permitSourcePackageFormat() should go.

 * Once that's done, you should remove the __init__ on the SourcePackageFormat model class and make the new utility's add() method initialise the correct fields.

Everything else looks great, thanks!