Code review comment for lp:~flacoste/launchpad/bug-801233

On 11-07-11 03:43 PM, Brad Crittenden wrote:
> Review: Approve code
> Hi Francis,
>
> This is a really good follow up branch to the changes I made. Thanks for making the processor traversal by name.
>
> I only found a few things to note:
>
> * As we discussed on IRC the docstring for getBuildQueueSizes make the wadl generator unhappy.

Yeah, this was a simple matter of using the rst literal block directive
to make it happy. (::)

This introduces a few other drive-bys:

* Turned out that our WADL to HTML conversion excludes the RST
parameters table (because we don't want the :param name: documentation
to be redundant or misleading with the web-service specific
documentation.) So I added support for the :return: (and :raise:)
parameters for custom methods (when appropriate). This will make the
return value of a couple other JSON-returning method documentated.

* Got bored and tired of seeing so many Unkonwn URL entry messages, so I
fixed a bunch. Not all of them are fixed. Will eventually chase the
others, but it's still an improvement!

>
> * In lib/lp/services/webservice/json.py you should add StrJSONSerializer to __all__.

Done.

>
> * In lib/lp/testing/factory.py, s/same name than the family./same name as the family./
>

Done.

> * Some of the lint is real, including new code with super long lines such as in test_default_collection.

Branch is now lint-free!

>
> * in ProcessorSet getAll and getByName why don't you use the storeOf(self)?

Because self isn't a DB instance (and thus doesn't have a store).
(That's usually the case in *Set object.)

Thanks for the review!
--
Francis J. Lacoste
<email address hidden>