[Merge] lp:~flo-fuchs/mailman.client/settings into lp:mailman.client
Florian Fuchs has proposed merging lp:~flo-fuchs/mailman.client/settings into lp:mailman.client. Requested reviews: Barry Warsaw (barry) For more details, see: https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Hi, I have added methods to delete mailing lists as well as a _Settings class to read and write list settings. Please feel free to make any changes you want (...and, of course, tell me if you feel this should be done completely differently... ;-) Cheers Florian -- https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Your team Mailman Coders is subscribed to branch lp:mailman.client.
Review: Approve This looks good, thanks! I have just two minor suggestions. * Where you're returning a unicode string object in the doctests, it would be better to use 'print' so you don't get the ugly u'' prefixes. E.g., instead of:
settings_new['description'] u'A very meaningful description.'
use:
print settings_new['description'] A very meaningful description.
* You should not need to use the u'' strings in your doctests. Mailman does a 'from __future__ import unicode_literals' in all its .py files to force literals to be unicodes (in the absence of explicit b'' prefixes). mailman.client should do the same. To make this work in doctests, you need to set the testobj's globs in the doctest's setUp(). See src/mailman/tests/test_documentation.py down about line 183 for an example. Other than that, it looks great, and please do merge it after consideration of the above. Thanks! -- https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Your team Mailman Coders is subscribed to branch lp:mailman.client.
The proposal to merge lp:~flo-fuchs/mailman.client/settings into lp:mailman.client has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 -- https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Your team Mailman Coders is subscribed to branch lp:mailman.client.
Review: Resubmit Hi, I've added a few changes to make the client compatible with the current Mailman3 alpha7. Member resources now use IDs instead of email adresses (on the API side). The client doesn't know this ID when provided with an email adress (for example to get membership information or to unsubscribe an email address from a list). In order to find the right ID, the client needs to iterate over the existing member roster. This probably isn't ideal, especially when lists have a large number of members. But it's probably not an issue that the client lib can solve differently... Thanks! Florian -- https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Your team Mailman Coders is subscribed to branch lp:mailman.client.
Review: Approve This looks great; a couple of comments: * See above for recommendations against u'' strings and for using print instead of returning strings directly in doctests. * Could you please open a bug (tagged with 'mailman3') on the need to expose an API call to look up a member-id given an email address? I'll make sure to expose that in a8. With the above consideration, I'll approve this change. Thanks! And feel free to land it. -- https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Your team Mailman Coders is subscribed to branch lp:mailman.client.
Review: Needs Information Hi, after merging the changes locally LP doesn't let me push to lp:mailman.client ("readonly transport"). Maybe that's because the approved revision is 14 while the branch is currently at rev. 16? Thanks! Florian -- https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Your team Mailman Coders is subscribed to branch lp:mailman.client.
I think this is Launchpad's way of saying you didn't have push permissions. Have you ever been able to push to this branch? Anyway, I added ~flo-fuchs to ~mailman-coders, which owns the trunk, so please try again. (P.S. I first accidentally added your doppleganger :) -- https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Your team Mailman Coders is subscribed to branch lp:mailman.client.
The proposal to merge lp:~flo-fuchs/mailman.client/settings into lp:mailman.client has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 -- https://code.launchpad.net/~flo-fuchs/mailman.client/settings/+merge/56088 Your team Mailman Coders is subscribed to branch lp:mailman.client.
participants (3)
-
Barry Warsaw -
Florian Fuchs -
noreply＠launchpad.net