launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

> 
> After reading the code, I think the branch creation rule is:
>     When The branch sharing policy is EMBARGOED_OR_PROPRIETARY,
>     only users granted PROPRIETARY may create branches
>     and the branch information type is EMBARGOED.
> 
> Thus both ~oem-solutions-group and ~project-team can both create branch, and those branches will be EMBARGOED. Members of ~oem-solutions-group and ~pmtteam will see all branches because all EMBARGOED is shared with them, but ~project-team can only see the branches they are subscribed to, which will be the branches they push.
> 
> ^ This is sound, but is my interpretation correct?
>

Yes. That matches my understanding and what I attempted to implement.


> I do not think we need to import COMMERCIAL_ADMIN_EMAIL on line 38. I don't intend to setup OEM's projects..they will. So I think
>         product.setBranchSharingPolicy(
>             BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY, comadmin)
> should be
>         product.setBranchSharingPolicy(
>             BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY, product.owner)
> to ensure the people who will do this can do so.
> 

Currently, only admins can set branch sharing policies right now.

> Per William's remarks we want to filter out Embargoed on lines 85 and 85 to ensure it does not show up in the UI.
>

Done.

> I am also uncomfortable with the use of with admin_logged_in() on line 140 because I don't think the project shares with ~admins (and I am certain they wont be doing what is demonstrated in the test). We could explicitly check this case using a user that is only shared PROPRIETARY information, but the product owner is also a valid actor.
>     with person_logged_in(namespace.product.owner):
> 

I copied existing code for this test but have fixed all three
occurrences in the test module.

-- 
https://code.launchpad.net/~wallyworld/launchpad/embargoed-information-type-1036187/+merge/119837
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.