Review: Needs Fixing Hi Varun,
thank you for doing another merge request! I have a couple of small issues, all concerning the ``user_delete`` view (some of them were already mentionend in Stephen Turnbull's comment on your last merge proposal): 1) ``except MailmanAPIError`` covers ``MailmanUser.objects.get`` (like it should), but doesn't cover ``user.delete()``. But it's possible (even if unlikely) that the REST service is running when you retrieve the user object, but isn't any more once you attempt to delete the user. Is the extra nesting necessary? 2) I think you can get rid of one of the two user_id assignments. Actually, I think you can get rid of both: We know from the url definition that this view function should only be called with a user_id argument. So it could be made a mandatory argument, instead of assigning it from **kwargs. If (for some reason) the function will be called *without* the user_id argument, the resulting TypeError is exactly what we want. 3) In an effort to make the try block as simple as possible, the ``messages.success`` call could probably be moved outside. 4) There are some minor PEP8 violations (argument list: no whitespace after ','; email_id assignment: no whitespace around '='). Cheers Florian -- https://code.launchpad.net/~varun/postorius/postorius_gsoc2013_modified/+merge/164490 Your team Mailman Coders is subscribed to branch lp:postorius. _______________________________________________ Mailman-coders mailing list [email protected] http://mail.python.org/mailman/listinfo/mailman-coders
