Code review comment for lp://staging/~bac/launchpad/bug-410416

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Brad. Thank you for this excellent test. It does a lot to clarify what private teams
can do. I have some trivial suggesting to improve this branch.

I have a question about the very part of the test. Since you are just documenting
existing behaviour in this case, I do not think we need to change anything at this time.

> === modified file 'lib/lp/registry/doc/private-team-roles.txt'
> --- lib/lp/registry/doc/private-team-roles.txt 2009-08-03 01:41:07 +0000
> +++ lib/lp/registry/doc/private-team-roles.txt 2009-08-10 17:08:27 +0000

...

Use two leading blank lines to separate a heading from the previous content.

> +Maintainer/Owner
> +----------------
> +
> +A public team and a private team can be a project owner but a private membership team cannot.

Wrap the narrative at 78 characters.

> +
> + >>> # The registrant must be specified or it will default to the owner.
> + >>> product = factory.makeProduct(registrant=admin_user, owner=public_team)
> + >>> product = factory.makeProduct(registrant=admin_user, owner=priv_team)
> +
> + >>> product = factory.makeProduct(registrant=admin_user, owner=pm_team)
> + Traceback (most recent call last):
> + ...
> + PrivatePersonLinkageError: Cannot link person
> + (name=private-membership-team, visibility=PRIVATE_MEMBERSHIP) to
> + <Product at...
> +
> +
> +Driver
> +------
> +
> +A public team and a private team can be a project driver but a private membership team cannot.

Wrap the narrative at 78 characters.

...

> + <Product at...
> +
> +Product Series Roles
> +====================

Use two leading blank lines to separate a heading from the previous content.

> +Owner
> +-----
> +
> +A public team and a private team can be a product series owner but a
> +private membership team cannot.
> +
> + >>> product = factory.makeProduct(registrant=admin_user, owner=public_team)

Is this line 79 characters?

...

> +Driver
> +------
> +
> +A public team and a private team can be a product series driver but a
> +private membership team cannot.
> +
> + >>> product = factory.makeProduct(registrant=admin_user, owner=public_team)

Is this line 79 characters?

...

> +Team Membership
> +===============
> +
> +Mixing public and private teams can create interesting situations.
> +Not every type of team can become a member of other types.
> +
> + >>> from lp.registry.interfaces.person import PrivatePersonLinkageError
> + >>> reviewer = factory.makePerson()
> + >>> def join_team(joined_type, joiner_type):
> + ... joined = factory.makeTeam(owner=team_owner, visibility=joined_type)
> + ... joiner = factory.makeTeam(owner=team_owner, visibility=joiner_type)

Wrap the code at 78 characters.

...

> + Public <- Public: Allowed
> + Public <- Private Membership: Not Allowed
> + Public <- Private: Not Allowed
> + ---
> + Private Membership <- Public: Allowed
> + Private Membership <- Private Membership: Not Allowed
> + Private Membership <- Private: Not Allowed
> + ---
> + Private <- Public: Allowed

Really? Is my team really private if the owner of the public team adds every
team he can find on launchpad? Is there some other guard in place to prevent
a the public team from exposing the private? A waring to the private team
that is is risking exposure by adding a public team that it does not control?

> + Private <- Private Membership: Not Allowed
> + Private <- Private: Not Allowed
> + ---

review: Needs Fixing (code)

« Back to merge proposal