Code review comment for lp:~intellectronica/launchpad/oops-462742

2009/10/28 Björn Tillenius <email address hidden>:
> On Wed, Oct 28, 2009 at 06:21:10PM -0000, Tom Berger wrote:
>> 2009/10/28 Björn Tillenius <email address hidden>:
>> > On Wed, Oct 28, 2009 at 06:05:27PM -0000, Tom Berger wrote:
>> >> === modified file 'lib/canonical/launchpad/database/structuralsubscription.py'
>> >> --- lib/canonical/launchpad/database/structuralsubscription.py        2009-10-23 13:51:40 +0000
>> >> +++ lib/canonical/launchpad/database/structuralsubscription.py        2009-10-28 18:05:23 +0000
>> >> @@ -331,9 +331,10 @@
>> >>          """See `IStructuralSubscriptionTarget`."""
>> >>          bug_subscriptions = self.getSubscriptions(
>> >>              min_bug_notification_level=BugNotificationLevel.METADATA)
>> >> -        for subscription in bug_subscriptions:
>> >> -            if (subscription.subscriber == user or
>> >> -                user.inTeam(subscription.subscriber)):
>> >> -                # The user has a bug subscription
>> >> -                return True
>> >> +        if IPerson.providedBy(user):
>> >
>> > What different kinds of objects can 'user' be?
>>
>> It can be None, but I felt that writing the condition like this made
>> it clearer. Would you have preferred comparing it to None instead?
>
> It's common to compare explicitly to None. If I see someone using
> IFoo.providedBy(object), I assume that object can be provide some other
> interface that IFoo.

Yes, that makes sense. Changed.