Code review comment for lp:~salgado/lazr.restful/extension-interfaces

Revision history for this message
Leonard Richardson (leonardr) wrote :

I didn't like "extends" for two reasons. First, "Foo extends bar" implies that 'foo' derives from 'bar', where as in this case 'bar' derives from 'foo'. Second, "extends" already means something in object-oriented programming and I didn't want to overload the term.

This is really good, almost ready to land. Comments:

I second Gary's suggestion about ConflictInContributingInterfaces and think your patch looks good.

You still need to describe this change in the NEWS file.

I don't know how useful that TestWebServiceConfiguration refactoring is, but it's fine since oyu use it twice. The register_module refactoring is a great idea.

I would like to see this feature used in an example web service (ie. added to examples/base or in a totally new web service of your design). This is less for purposes of test coverage and more to show how the feature would work in a real situation.

Lines 802 and 803 are the same.

Some of your tests are missing explanatory comments. The stub classes like Product need to have docstrings saying things like "A stub class that publishes one string field and one read operation."

review: Needs Fixing

« Back to merge proposal