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

Revision history for this message
Gary Poster (gary) wrote :

I've done a non-exhaustive review (as a supplement to Leonard's not a replacement), and want to share my comments so far.

Requests:

- This is an important new feature for lazr.restful, and I don't see any instructions on how to use it. Could you add some documentation (doctest) to the example webservice part of the package? (As a small corollary, we'll want to mention something in the CHANGES/NEWS file.)

- I see that find_interfaces_and_contributors is responsible for raising ConflictInContributingInterfaces. I'd like to make sure that these errors are as helpful as they can be. Right now the exception just has the name and the base interface that has been modified. I'd also like to see the two or more interfaces that have created the conflict--and I'd like a test that shows that a traceback with this error gives the developer a nice error message telling what's gone wrong and where to look to address it. (A unit test of the error's __str__ should be sufficient.)

Suggestions:

- for the new argument name, "contributes_to" -> "extends"? Take it only if you like it, but it matches nicely with "extensions," which you use later, for instance. If you do this, you'd want to change "contributors" to "extenders," I suspect.

« Back to merge proposal