Thanks for the review! On Thu 14 Jun 2012 14:45:23 EST, William Grant wrote: > Review: Approve code > > 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. >
I'll add it pillar filtering prior to landing. > 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. Looks good I think. > > 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? > The first one can. The second won't fit. To me it looked better if they were both done the same way. But I've changed it. > 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 :) > I think you misread it :-) It's pulling serialised values (the names) from the stored json and looking them up in an items list associated with the InformationType.name attribute. > 495 +# def test_revokeAccessGrantsBranches(self): > > 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. The test worked fine in the previous branch. I had left it in as a reminder so it could be adapted when the next job was added but will remove it. -- https://code.launchpad.net/~wallyworld/launchpad/unshare-team-members-subscriptions2-1009358/+merge/110215 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
