Code review comment for lp:~michael.nelson/launchpad/db-598464-priv-xmlrpc-access-getOrCreate

Revision history for this message
Abel Deuring (adeuring) wrote :

(13:09:31) adeuring: noodles775: again, a nice branch. But I think it is a bit odd to have a class PersonSetAPIView (ie.e.,a quite generic name) where the method getorCreateByOpenIDIdentifier() unconditionally providesthe rationale "software centerpurchase" as a account creation rationale. Shouldn't we either make the class name more specific or provide the rationale as a parameter of the method?
(13:10:47) adeuring: (or use a method name like getOrCreateForSoftwarePurchase())
(13:45:12) noodles775: adeuring: yes - I thought the same when I pushed the last rev. So I'd be keen to change the application name... most of the other private xmlrpc apps have names like 'ICodehostingApplication' identifying who uses them.
(13:46:42) noodles775: So perhaps, ISoftwareCenterAgentAPI, http://..../softwarecenteragent etc.?
(13:47:07) adeuring: noodles775: ok, so something like s/Person/Shopper/ ?
(13:48:29) noodles775: The app name usually matches the API, so perhaps ISoftwareCenterAgentApplication...
(13:50:06) adeuring: noodles775: ah, right. so, that, combined with a method name like getOrCreateShopCustomer()?
(13:50:20) noodles775: Sounds good.

review: Approve (code)

« Back to merge proposal