launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

> 1. I think the preservation of existing driver subscriptions is good.
>    Maybe the code can be DRY if `required_subscribers.add(pillar.driver)`
>    was in the ` if information_type in PRIVATE_INFORMATION_TYPES` block

Sure, that's doable; it also needs the !ubuntu check on it.

> 2. Is the bug reporter and person making the information type change
>    also subscribed and given an access grant?

They are; line 8 of the current diff.

> 3. Is there a test that shows that the reporter, changer, and driver
>    have access?

Yes--those are the modified tests in test_bug. They don't directly test access, as they began as subscriber checks, but as you can only be subscribed to a bug you have access to on a private change, the function is tested. We could update those tests to explicitly check for access, if you think we no longer need any subscriber tests; if we need to preserve those tests I'm hesitant to add more as they would be essentially identical checks.
-- 
https://code.launchpad.net/~jcsackett/launchpad/remove-bad-subscribers/+merge/117771
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.