Review: Approve
post landing review - some thoughts: getSubscriptionForPerson seems to overlap 
other getSubscription methods quite a bit. One particular thing is that this 
seems buggy: I can subscribe my *team* but the edit facility will only let me 
edit my *personal* subscription: its a contract mismatch between the 
subscribe-and-edit areas.

This repeated condition:
 if self._use_advanced_features:

looks like it would be worth factoring out into a separate object that we 
delegate too.

As a reviewer I wouldn't have blocked this, because its for an indevelopment 
feature, and I think the getSubscriptionForPerson mismatch is improvable in 
subsequent landings. OTOH perhaps this would never have been an issue if it had 
been talked around earlier (before getting to this point of code).
-- 
https://code.launchpad.net/~gmb/launchpad/add-update-subscription-workflow-bug-664571/+merge/39966
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~gmb/launchpad/add-update-subscription-workflow-bug-664571 into 
lp:launchpad/devel.

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to