launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

> 
> One of these is: the "Or" in the vocab's central query is a performance
> hazard. "OR" of ten is. The TeamParticipation table makes everyone a member of
> themselves, so the transitive-ownership check will also show all direct-
> ownership cases. Yet the query plan probably won't be able to avoid fetching
> data for the transitive check, so you may be getting one query for the price
> of two.
> 
> The other is: the change is pretty fundamental to the vocabulary. If the
> parameter you're passing is always going to be a literal, why not make this a
> separate vocab and use inheritance to express the commonality? It also means
> you don't have to kludge around the difference in query conditions.
> 
> Putting these two together, I would say: keep the current dictionary as it is.
> Create a separate one that's generalized for team-owned branches. It's going
> to be faster and easier to read, and maybe someday we'll decide that we don't
> actually need the direct-ownership one as much as we thought.


I like the suggested changes. I didn't realise that everyone was a member of themselves. 
I've added a new method to IBranchCollection - ownedByTeamMember. This makes the API easier to understand. The only use of the HostedBranchRestrictedOnOwner vocab that I can see is the gui for choosing the product series translation branch so I don't see a need to subclass the vocab at this stage. 
-- 
https://code.launchpad.net/~wallyworld/launchpad/branch-picker-team-branches/+merge/52356
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/branch-picker-team-branches into lp:launchpad.