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

Hi Leonard,

This looks good to me. I have some small fixme's:

diff line 36: param should be "versioned_annotations" not "annotations"
diff line 122: you define stack = entry_tags.stack presumably to make less typing below but then forget to use stack
Perhaps you could tidy up the zope.security.management imports as a drive by in test_utils? The ones in test_declarations also need some lovin' if you are so inclined - there's unused ones as well as formatting issues.

A concern/question:

>> def normalize_for_versions(self, versions, default_dictionary={}, error_prefix=''):

The default_dictionary argument defaults to {} but this is mutable. This can lead to unintended side effects since default args are only evaluated once when the function is defined. It doesn't look like anything bad happens in this case but my preference is to use default_dictionary=dict() just to be safe and explicitly correct in all cases.

A question:

In normalize_for_versions(self, versions, ...), do the later, more recent versions appear towards the front of the versions list? Perhaps the docstring could mention this just to clarify since being unfamiliar with the implementation, it was unclear to me without trying to figure it out.

A suggestion:

In places, instead of using 2-tuple's, eg versioned_annotations parameter to export_as_webservice_entry(), you could consider using a namedtuple. This will make code like:

    if entry_tags.stack[0][0] is None:
        entry_tags.stack[0] = (earliest_version, entry_tags.stack[0][1])

much easier to read, especially for people not previously 100% familiar with the code. It would become something like:

    if entry_tags.stack[0].version is None:
        entry_tags.stack[0] = (earliest_version, entry_tags.stack[0].annotations)

Also the return value of generate_entry_interfaces. So

interfaces = generate_entry_interfaces(...)
... interfaces[0][0]
... interfaces[0][1]

becomes

interfaces = generate_entry_interfaces(...)
... interfaces[0].version
... interfaces[0].interface