launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing code



Diff comments:

> === modified file 'lib/lp/bugs/mail/bugnotificationbuilder.py'
> --- lib/lp/bugs/mail/bugnotificationbuilder.py	2012-09-19 01:19:35 +0000
> +++ lib/lp/bugs/mail/bugnotificationbuilder.py	2015-01-09 07:44:27 +0000
> @@ -156,6 +156,9 @@
>                      '%s (%s)' % (event_creator.displayname,
>                          event_creator.name)))
>  
> +        # Add the notification -type header.
> +        self.common_headers.append(('X-Launchpad-Notification-Type', 'bug'))

The comment is inconsistent with the rest, and I'd consider moving it to the top with the other generic headers.

> +
>      def build(self, from_address, to_address, body, subject, email_date,
>                rationale=None, references=None, message_id=None, filters=None):
>          """Construct the notification.
> 
> === modified file 'lib/lp/bugs/mail/tests/test_bugnotificationbuilder.py'
> --- lib/lp/bugs/mail/tests/test_bugnotificationbuilder.py	2012-01-01 02:58:52 +0000
> +++ lib/lp/bugs/mail/tests/test_bugnotificationbuilder.py	2015-01-09 07:44:27 +0000
> @@ -50,3 +50,11 @@
>          self.assertContentEqual(
>              [u"Testing filter", u"Second testing filter"],
>              message.get_all("X-Launchpad-Subscription"))
> +
> +    def test_mails_contain_notification_type_healder(self):
> +        utc_now = datetime.now(pytz.UTC)
> +        message = self.builder.build('from', 'to', 'body', 'subject',
> +                                     utc_now, filters=[])
> +        self.assertEqual("bug",
> +                      message.get("X-Launchpad-Notification-Type", None))

Typo in the test name, and

we = normally_format(
    method, calls, that, span, multiple, lines,
    like, this)

> +
> 


-- 
https://code.launchpad.net/~thomir/launchpad/devel-add-bug-header/+merge/245921
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.