Code review comment for lp:~wallyworld/launchpad/built-packages-listing

Revision history for this message
Steve Kowalik (stevenk) wrote :

Hi,

This mostly looks good. I have a few comments:

* Why are you including imports in the meat of the test, rather than with the rest of them at the top of the file?
* Would _makeRecipeBuildRecords() better exist in the factory so other tests could make use of it?
* Would it make sense to fix __eq__ on SourcePackageRecipe, rather than working around it?
* Use of IStoreSelector, which is deprecated. Could you look at switching to I{Master,}Store?
* Please update your XXX per the XXX policy.

review: Needs Fixing (code*)

« Back to merge proposal