Code review comment for lp:~jml/launchpad/show-warnings-once

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Because the warnings include repr()'s this doesn't help in a lot of situations, e.g.:

/var/launchpad/test/lib/lp/soyuz/tests/test_publishing.py:229: ShouldThisBeUsingRemoveSecurityProxy: removeSecurityProxy(<PackageUpload at 0x16f126d0>) called. Is this correct? Either call it directly or fix the test.
  package_upload)

/var/launchpad/test/lib/lp/soyuz/tests/test_publishing.py:229: ShouldThisBeUsingRemoveSecurityProxy: removeSecurityProxy(<PackageUpload at 0x146b2210>) called. Is this correct? Either call it directly or fix the test.
  package_upload)

I guess we should make sure to improve our __repr__ methods to fix more of these.

Either way, it's better than using a list. Should it perhaps be sorted so we see the warnings grouped per file, line?

review: Approve (code)

« Back to merge proposal