Code review comment for lp:~michael.nelson/launchpad/594492-present-bfjs-in-builder-history

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

A few more changes discussed on IRC:

 * The tests are much better, but hiding a login() in a helper is a bit obscure. Replaced with a removeSecurityProxy.
 * In test_binary_only_false, instead of just testing that builds.count() == 3 for a binary-only query, better to test separately that (1) all its elements are binary builds, (2) the result is nonempty so the test isn't trivial, and (3) the result is smaller than with a non-binary-only query so the test isn't meaningless.

Great shape now. Land that baby!

Quod subigo farinam,

Jeroen

review: Approve (code)

« Back to merge proposal