Code review comment for lp://qastaging/~rvb/maas/maas-api-doc

Revision history for this message
Raphaƫl Badin (rvb) wrote :

> [1]
> [2]
[...]
> doc: docs/api.rst
> $(MAKE) -C docs html

Done.

> [3]
>
> html-doc and api-doc - or just doc - need to be added to .PHONY.

Okay, what it this .PHONY thing anyway?

> [4]
> Wrap this please :)

Done.

> [5]
>
> +PISTON_IGNORE_DUPE_MODELS = True
>
> What problem is this solving? Add a comment perhaps?

I confess I'm not really sure, I'll investigate some more but like I said, I think it's fine (because I had a quick glance at the code and because the api seems to work).

> [6]
> This is a lot of screen space for not a lot. Perhaps put this in a
> single line? I quite like the way we do it in Launchpad, but I'm not
> wedded to it.

Good point.

> [7]
>
> + <h2>MaaS server Api Documentation</h2>
>
> API is a TLA, I think it should be written "API". Also, if
> "Documentation" is capitalized then I think "server" ought to be too,
> or vice-versa.

Done.

> + <table>
> + <thead>
> + <tr><td>Ressource</td><td>Description</td></tr>
>
> s/Ress/Res/

Arg!

> [8]
>
> + messages.append(
> + "%s %s\n %s\n\n" % (
> + method.http_name, doc.resource_uri_template,
> + method.doc))
>
> Does Piston make sure method.doc (and, to a lesser extent, the other
> things that get interpolated) is suitable for inserting into an reST
> doc? Is it assumed that we write the docs using reST? That's fine with
> me, and we should add it to HACKING.txt.

http_name is fine, and so is resource_uri_template. You're right, we need say in HACKING.txt that the docs for api methods should use reST.

« Back to merge proposal