Good branch, though there are some little warts that came up on IRC and
which you fixed interactively:
* Don't return None when looking up a malformed id if a well-formed id
that doesn't belong to any bug nomination reaises NotFoundError.
* Don't over-test isApproved() in the doctest, though it was fine as a
transitional check during development. It's already covered in
bug-nominations.txt.
* Better not export canApprove() for any person, because it may expose
private team memberships. Just make it apply to the current user:
"Can I approve this?"
* Test tearDown belongs just below setUp.
* Don't derive a test case from another, but lift the commonality up
into a mixin class.
* You inherited a few small pieces of lint; should be easy to clean up.
Well done!
Good branch, though there are some little warts that came up on IRC and
which you fixed interactively:
* Don't return None when looking up a malformed id if a well-formed id
that doesn't belong to any bug nomination reaises NotFoundError.
* Don't over-test isApproved() in the doctest, though it was fine as a nominations. txt.
transitional check during development. It's already covered in
bug-
* Better not export canApprove() for any person, because it may expose
private team memberships. Just make it apply to the current user:
"Can I approve this?"
* Test tearDown belongs just below setUp.
* Don't derive a test case from another, but lift the commonality up
into a mixin class.
* You inherited a few small pieces of lint; should be easy to clean up.
With that out of the way, r=me.
Jeroen