Code review comment for lp:~david-soto/mailman/postconf_equivalent

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

This branch looks great, and I'm really sorry for letting this sit for so long. I'm finally merging this at Pycon 2013, and thanks for your contribution to Mailman!

I made a few changes. I renamed it from Mailmanconf to Conf and I renamed the command from 'mailmanconf' to just 'conf' (the 'mailman' prefix part seemed redundant since this always gets run as a subcommand). I also added a few unittests for making sure that output to a file works, and I added "-o -" as a synonym for output to stdout. Also, TestConf must have a "layer = ConfigLayer" attribute otherwise the configuration system isn't set up by the time the tests run. Ironically this doesn't affect the failing tests, but with the expected-to-succeed tests I added, this has to be done.

I just cleaned up a few other things, but basically kept your code pretty well intact. Great work.

BTW, what do you think of adding a --sort/-s option so that the output is printed in sorted order, i.e. first by section and then by key?

If you like the idea, how would you like doing a new branch to add that feature?

review: Approve

« Back to merge proposal