launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Thanks for addressing my comments. I have a few new ones:

312	+from storm.expr import (

This should be in the third-party section, after the lazr imports.

371	+ # Determine the pillars via which an access policy grant will give
372	+ # access.
373	+ affected_pillars = []
374	+ if access_artifact.bug_id:
375	+ affected_pillars = concrete_artifact.affected_pillars
376	+ else:
377	+ target_context = concrete_artifact.target.context
378	+ if IPillar.providedBy(target_context):
379	+ affected_pillars.append(target_context)

Can't you run getPeopleWithoutAccess after _reconcileAccess, so you can just use AccessPolicyArtifact rather than duplicating the pillar determination logic?

403	+ AccessPolicyGrantFlat.abstract_artifact_id ==
404	+ access_artifact.id,
405	+ AccessPolicyGrantFlat.grantee_id ==
406	+ TeamParticipation.teamID),

This looks like it'd be fine with AccessArtifactGrant. No need for AccessPolicyGrantFlat.
-- 
https://code.launchpad.net/~wallyworld/launchpad/create-aag-privacy-transitions2-1009364/+merge/110706
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/create-aag-privacy-transitions2-1009364 into lp:launchpad.