Code review comment for lp:~bac/launchpad/bug-799901

Revision history for this message
Gavin Panella (allenap) wrote :

Tip top :)

[1]

+ if IBug.providedBy(self.context):
+ bug = self.context
+ elif IBugTask.providedBy(self.context):
+ bug = self.context.bug

I know you only moved this, but I thought I'd mention that there is an
adapter from IBugTask to IBug, so the above can be written as:

    bug = IBug(self.context)

[2]

+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)

Cool, I've not see that before.

[3]

+ # A subscriber_data_js returns JSON string of a list

That should probably read:

        # subscriber_data_js returns a JSON string of a list

or something like that.

[4]

In the subscriber dict, assembled in direct_subscriber_data,
subscribed_by /feels/ like it should be the person, rather than a
description. Consider doing instead something like:

            subscriber = {
                'name': person.name,
                [...]
                'display_subscribed_by': subscription.display_subscribed_by,
                'subscribed_by_link': absoluteURL(
                    subscription.subscribed_by, self.api_request),
                }

That makes the data more generally useful, and it more closely
resembles the BugSubscription API.

Ach, it's not important, just an idea.

review: Approve

« Back to merge proposal