Code review comment for lp:~wallyworld/launchpad/unshare-team-members-subscriptions2-1009358

As discussed on IRC, this branch makes the jobs completely unsuitable for product. Most notably, the jobs in a lot of cases end up verifying the subscription consistency of hundreds of thousands or millions of unrelated bugs. However, it's a good intermediate step, since the jobs remain disabled on production.

Since it doesn't make a huge amount of sense to review the functionality, there's just a couple of style comments. In addition to the below, I also propose http://pastebin.ubuntu.com/1040252/ as a bit of a cleanup around the admin special case.

185 bug_ids = [
186 - bug.id for bug in bugs
187 + bug.id for bug in bugs or []
188 + ]
189 + information_types = [
190 + info_type.value for info_type in information_types or []
191 ]

Can't those list comprehensions be on one line each?

202 + @property
203 + def information_types(self):
204 + return [
205 + enumerated_type_registry[InformationType.name].items[value]
206 + for value in self.metadata['information_types']]

Why does this use the registry? You are looking the enum up by the name you got from the enum :)

495 +# def test_revokeAccessGrantsBranches(self):
496 +# # Users with launchpad.Edit can delete all access for a sharee.
497 +# owner = self.factory.makePerson()
498 +# product = self.factory.makeProduct(owner=owner)
499 +# login_person(owner)
500 +# branch = self.factory.makeBranch(
501 +# product=product, owner=owner,
502 +# information_type=InformationType.USERDATA)
503 +# self._assert_revokeTeamAccessGrants(distro, [bug], None)

I'm not sure a commented test is useful, particularly when we have the work planned and the test won't ever work as written.