launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Review: Needs Information

Like the implementation of getAllowedInformationTypes() on IBranch, should we allow admins to access to all information types? If not, I'd like to see a comment in the docstring stating why the user parameter is ignored. Also, the interface calls the parameter 'user', the class calls it 'who'. These should be consistent.

It is really the case that there's no model support for Proprietary? If a project is a commercial project, then Proprietary is allowed isn't it? (so long as the feature flag allows it; well, assuming the feature flag is not deleted in this branch). If we take this approach, then there's no need to delete an number of the tests. I'd rather not delete code only to add it back later.

With the removal of the items from the json cache for +filebug on project groups, did you test this this fully locally? I can't recall if there was an issue at one stage with not having the cache fully populated, perhaps there wasn't, I'm not sure.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-informationtype-refactor/+merge/117002
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.