Code review comment for lp:~leonardr/lazr.restful/entry-introduced-in-version

Revision history for this message
Tim Penhey (thumper) wrote :

Does lazr.restful use testtools for the TestCase?

If it does, assertRaises returns the exception, so
test_non_existent_annotation_fails could be somewhat
simplified.

It seems that normalize_for_versions is only passed
no dict through in tests. I believe the more used
approach is to have the default value to be None, and
have something like:

  if default_dictionary is None:
    default_dictionary = {}

I wish we had a way to have contracts on the params.
In C++ it would be a "const" dict. Here though we are
doing a deep copy. My preference, even though you don't
mutate the dict now, would be to use None as the param.

review: Needs Fixing (code)

« Back to merge proposal