Code review comment for lp:~jtv/launchpad/bug-844550

Revision history for this message
Graham Binns (gmb) wrote :

Hi Jeroen,

Excellent branch this; I don't have any questions other than:

112 + self.assertEqual([
113 + PackagePublishingStatus.SUPERSEDED,
114 + PackagePublishingStatus.SUPERSEDED,
115 + PackagePublishingStatus.PUBLISHED,
116 + PackagePublishingStatus.DELETED,
117 + PackagePublishingStatus.PENDING,
118 + ],
119 + [pub.status for pub in spphs])

This might be more comprehensible if it were written as single statements (though I can appreciate that that might be a bit less elegant), purely from a "don't make me think about it" point of view. Also, if it's a series of single asserts, it's likely to be simpler to unwind when something goes wrong.

review: Approve (code)

« Back to merge proposal