launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code

32	+def convert_to_information_type(private):
33	+ if private:
34	+ return InformationType.USERDATA
35	+ else:
36	+ return InformationType.PUBLIC

I'd put this in model rather than adapters. It's not useful outside model.

101	+class BranchCannotChangeInformationType(Exception):
102	+ """The branch cannot change its information type."""

Of course it can't -- a branch isn't an actor. Do you mean that its information type can't be changed?

116	+from lazr.restful.interface import copy_field

Unused?

159	+ @property
160	def private(self):
161	- return self.transitively_private
162	+ return self.information_type in PRIVATE_INFORMATION_TYPES

Doesn't this need to defer to transitively_private if information_type? Depending on the way information_type is populated later, we may even need to ignore information_type completely until the migration is done.

174	+ if (
175	+ self.stacked_on and self.stacked_on.information_type !=
176	+ InformationType.PUBLIC and information_type !=
177	+ self.stacked_on.information_type):

That is the worst line wrapping in the history of the universe.

211	+ if (
212	+ self.stacked_on and self.stacked_on.information_type !=
213	+ InformationType.PUBLIC):
214	+ self.information_type = self.stacked_on.information_type

This could do with one fewer line breaks, and possibly a comment.

327	+ if information_type:
328	+ removeSecurityProxy(branch).information_type = information_type
329	if private:
330	removeSecurityProxy(branch).explicitly_private = True
331	removeSecurityProxy(branch).transitively_private = True
332	+ if not information_type:
333	+ removeSecurityProxy(branch).information_type = (
334	+ InformationType.USERDATA)

Should you perhaps merge the two information_type bits? And how close are we to eliminating makeBranch(private)?
-- 
https://code.launchpad.net/~stevenk/launchpad/branch-use-information_type/+merge/106109
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.