Code review comment for lp:~al-maisan/launchpad/ppa-visibility-514824

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

The conversation so far:

<allenap> al-maisan: In xx-distribution-packages.txt, the disabled PPA is created, and it's absence from the PPA list is the test. Can you add a comment along the lines of "The disabled PPA is not shown in the page". Or, better, demonstrate that it is shown, disable it, and demonstrate that it is no longer shown.
<al-maisan> allenap: I'll add the comment if you don't mind. Our test suite already takes too much time to run :)
<allenap> al-maisan: Okay :)
<allenap> al-maisan: Also, ViewSourcePackagePublishingHistory could be changed to just inherit from ViewArchive, with a custom __init__() that calls the superclass with obj.archive.
<al-maisan> allenap: good point .. I'll look into that.
<allenap> al-maisan: ViewArchive.checkAuthenticated() already checks for user.is_admin, so both ViewSourcePackagePublishingHistory.checkAuthenticated() and ViewSourcePackagePublishingHistory.checkUnauthenticated() both defer entirely to ViewArchive.
<al-maisan> allenap: right.
<allenap> al-maisan: You don't need to change that though; the composition is quite understandable.

« Back to merge proposal