Code review comment for lp:~pcjc2/launchpad/allow-empty-comments

Hi Peter,

I'm really pleased with this branch - nice work! I've got a couple of stylistic nitpicks but it's nothing major. Once those are fixed I'll push it into our merge machinery for you.

9 + # If there is no attachment and no comment text, use a place-holder
10 + no_attachments = get_element (commentnode, 'attachment') == None

It took me a couple of seconds to parse this. I think it would be better as part of the conditional that follows it, thus:

    # If there is no attachment and no comment text, use a place-holder.
    if (get_element(commentnode, 'attachment') is None and
        (text is None or text == '')):
        ...

66 + self.assertEqual(message4.owner.preferredemail.email,
67 + '<email address hidden>')
68 + self.assertEqual(message4.datecreated.isoformat(),
69 + '2005-01-01T14:00:00+00:00')
70 + self.assertEqual(message4.subject, 'Re: A test bug')
71 + self.assertEqual(message4.text_contents, '<empty comment>')
72 + self.assertEqual(message4.bugattachments.count(), 0)
73 +
74 + # Message 5:
75 + self.assertEqual(message5.owner.preferredemail.email,
76 + '<email address hidden>')
77 + self.assertEqual(message5.datecreated.isoformat(),
78 + '2005-01-01T15:00:00+00:00')

I know that all the other asserts in the test are formatted like this, and I don't expect you to correct those too (I'll file a bug about cleaning it up), but these should be formatted as I said in my other review:

    self.assertEqual(
        message5.datecreated.isoformat(), ...)

And so on.