launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

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.