Code review comment for lp:~al-maisan/launchpad/unembargo-443075

Hi Muharem,

On 09.10.2009 17:57, Muharem Hrnjadovic wrote:
> Abel Deuring wrote:
>> Review: Approve
>> Hi Muharem,
>>
>> nice work, just a few cosmetic remarks, see below.
>
> Hello Abel,
>
> I am thanking you for the prompt review. All your suggestion were
> accommodated except for one. Please see the enclosed incremental diff as
> well as my replies below for more detail.
>
>> Abel
>>
>>> === modified file 'lib/lp/soyuz/tests/test_packageupload.py'
>>> --- lib/lp/soyuz/tests/test_packageupload.py 2009-09-29 17:16:01 +0000
>>> +++ lib/lp/soyuz/tests/test_packageupload.py 2009-10-09 09:50:27 +0000
>>> @@ -198,6 +198,9 @@
>>> self.assertEquals(
>>> PackageUploadStatus.ACCEPTED, delayed_copy.status)
>>>
>>> + # Make sure no announcement email was sent at this point.
>>> + self.assertEquals(len(stub.test_emails), 0)
>>> +
>>> self.layer.txn.commit()
>>> self.layer.switchDbUser(self.dbuser)
>>>
>>> @@ -210,22 +213,26 @@
>>>
>>> # Check the announcement email.
>>> from_addr, to_addrs, raw_msg = stub.test_emails.pop()
>>> - # This is now a MIMEMultipart message.
>>> msg = message_from_string(raw_msg)
>>> body = msg.get_payload(0)
>>> body = body.get_payload(decode=True)
>>>
>>> - self.assertEquals(from_addr, '<email address hidden>')
>>> self.assertEquals(
>>> - to_addrs, ['<email address hidden>'])
>>> + str(to_addrs), "['<email address hidden>']")
>>>
>>> expected_subject = (
>>> '[ubuntutest/breezy-autotest-security]\n\t'
>>> 'dist-upgrader_20060302.0120_all.tar.gz, foocomm 1.0-2 (Accepted)')
>>> self.assertEquals(msg['Subject'], expected_subject)
>>>
>>> - self.assertTrue(body.startswith('foocomm (1.0-2) breezy; urgency=low'))
>>> -
>>> + self.assertEquals(body,
>>> + 'foocomm (1.0-2) breezy; urgency=low\n\n'
>>> + ' * Initial version\n\n'
>>> + 'Date: Thu, 16 Feb 2006 15:34:09 +0000\n'
>>> + 'Changed-By: Foo Bar <email address hidden>\n'
>>> + 'Maintainer: Launchpad team <email address hidden>\n'
>>> + 'http://launchpad.dev/ubuntutest/breezy-autotest/+source/foocomm/1.0-2\n')
>> This line is a bit too long. Could you split itto two lines? (OK,
>> breaking the URL into two line does ot look nice either...
>
> With your permission I'd like to leave it as it is.

Hrmm, OK... I am not that sure but let's leave it.

Abel