566714 - update api-utils.md to apidocs format

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Noelle Murata]

currently in simple markdown format, moving to apidocs format.

Comment 1

[Noelle Murata]

Attachment: api utils doc to apidocs format

Comment 2

[Noelle Murata]

Comment on attachment 446066 [details] [diff] [review]
api utils doc to apidocs format 

Myk, can you make sure that I did the patch process correctly before I have Drew review it?

Comment 3

[Drew Willcoxon :adw]

Comment on attachment 446066 [details] [diff] [review]
api utils doc to apidocs format 

In the spirit of pitching in with reviews as mentioned in today's meeting, I can look at this.

Comment 4

[Drew Willcoxon :adw]

Comment on attachment 446066 [details] [diff] [review]
api utils doc to apidocs format 

Thanks Noelle, this looks good, just a few things.

>+<!-- contributed by Drew Willcoxon [[e-mail]]  -->
>+<!-- edited by Noelle Murata [[e-mail]]  -->
> The `api-utils` module provides some helpers useful to Jetpack's high-level API

Please add a blank line between the comments and this first sentence to increase readability.

>+<api name="publicConstructor">
>+@method
>+Returns a function C that creates an instance of `privateCtor`. *C*

C is emphasized with asterisks (e.g., *C*, like the second use in this sentence and elsewhere), so the first use in this sentence also needs asterisks.

>+@param [options] {object} The options dictionary to validate.  It's not 
>+modified. If it's null or otherwise falsey, an empty object is assumed.
>+@param [requirements] {object} An object whose keys are the expected keys in 
>+*`options`*. Any key in *`options`* that is not present in *`requirements`* is 
>+ignored.  Each value in *`requirements`* is itself an object describing the 
>+requirements of its key.
>+@prop [map] {function} A function that's passed the value of the key in the 
>+*`options`*. `map`'s return value is taken as the key's value in the final 
>+validated options, `is`, and `ok`. If `map` throws an exception it is caught and 
>+discarded, and the key's value is it's value in `options`.

it's -> its

>+@prop [is] {array} An array containing the number of `typeof` type names. If the 
>+key's value is none of these types it fails validation. Arrays and nulls are 
>+identified by the special type names "array" and "null"; "object" will not match 
>+either. No type coercion is done.
>+@prop [ok] {function} A function that is passed the key's value. If it returns 
>+false, the value fails validation.
>+@prop [msg] {string} If the key's value fails validation, an exception is
>+thrown. This string will be used as it's message. If undefined, a generic

it's -> its

>+message is used, unless `is` is defined, in which case the message will state
>+that the value needs to be one of the given types. 
>+</api>

It's difficult to read these lines in the markdown.  Since there's no spacing or indentation, they all kind of run together.  Could you come up with a nice way of making this kind of text more readable?

Nit: Many of the lines have an unnecessary space at the end of them.  It's not a big deal, but could you remove those spaces if possible?

Comment 5

[Noelle Murata]

Attachment: with requested fixes per adw

Comment 6

[Drew Willcoxon :adw]

This looks good, thanks.  One thing I just noticed that I should have mentioned before is, with these changes it's no longer clear that the @props of the requirements object are all optional.  (It's also no longer clear that the @props are @props at all, or how they are connected to the requirements object.  But that's probably more of a problem with the stylesheet.  Or maybe the type of requirements param should be {options dictionary} or something.  What do you think?)

So, this patch should be updated to make it clear that the @props are optional.  I can make the change if you want, or you can attach a new patch, let me know.

Comment 7

[Noelle Murata]

Attachment: with requested fixes per adw

Basically, the difference is really subtle. Required @params are indicated in the apidocs format as being contained by square brackets and displayed as italic text. We can revisit the style sheet at another point. I'll file a bug.

Comment 8

[Drew Willcoxon :adw]

Comment on attachment 446127 [details] [diff] [review]
with requested fixes per adw

Thanks Noelle.  Actually `requirements` is a required param, just its props aren't.  I'll go ahead and add brackets around it since it's a small change and push your patch.

(In reply to comment #7)
> Basically, the difference is really subtle. Required @params are indicated in
> the apidocs format as being contained by square brackets and displayed as
> italic text. We can revisit the style sheet at another point. I'll file a bug.

Yeah, that's subtle.  You're probably already planning for it, but a "conventions used in these docs" page would be really helpful -- once all docs are following some conventions of course. :)  Or in this case maybe just tiny "Optional" text next to the names.  Also, just offering this as some feedback, but in other contexts where brackets are used to indicate that something is required or not (e.g., command-line programs), they usually indicate *not*, i.e., that whatever they contain is optional.  (That might be a point I should raise with Brian I guess, since he wrote the doc parser?)

Comment 9

[Drew Willcoxon :adw]

Pushed: http://hg.mozilla.org/labs/jetpack-sdk/rev/09fb4f0aed00

Comment 10

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".