Code review comment for lp:~abompard/mailman/fix-import-from-mm2

Revision history for this message
Barry Warsaw (barry) wrote :

Accepted and merged, with some style changes:

* Single quotes for double quotes in several places.
* Full sentences in comments.
* No hanging comments.
* Refactored getUtility()
* Comments after test function name/signature.
* Long lines.
* Abbreviations.

I'm moderately concerned that the Bouncer class isn't tested explicitly; does this show up in real data or in the test sample data? Any chance you can write a follow up test for that? (It's fine that bounce info is thrown away during an import.)

I also added a context manager to ensure sys.modules gets cleaned up, and then I used an ExitStack in the with statement. These things are awesome. :)

review: Approve

« Back to merge proposal