566733 - update byte-streams.md to apidocs format

Status: RESOLVED FIXED

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

Description

[Noelle Murata]

updating byte-streams to apidocs format

Comment 1

[Noelle Murata]

Attachment: update byte streams

Comment 2

[Drew Willcoxon :adw]

Noelle, is this patch ready for review?

Comment 3

[Noelle Murata]

Please try it. I hope it's good, but I'm still working out some kinks in my mercurial workflow, so let me know if it isn't solid.

Thanks!

Comment 4

[Drew Willcoxon :adw]

Comment on attachment 446079 [details] [diff] [review]
update byte streams 

The patch applies cleanly, so as far as I can tell everything Mercurial on your end looks good.  A few things:

>+<!-- contributed by Drew Willcoxon [[e-mail]]  -->
>+<!-- edited by Noelle Murata [[e-mail]]  -->
> This module contains functionality for byte input and output streams.

Please add a blank line between the comments and the first sentence to make the markdown more readable.

>+The namespace for this API is `byte-streams`.

I haven't seen the word "namespace" used formally with Jetpack.  Do you know if (or are you planning that) we will start using it in the docs?  If not, it would be better to change the first sentence to something like "The `byte-streams` module contains functionality for..." and leave out the second sentence, to be consistent with the other docs.

>+<api name="ByteReader">
>+@constructor
> Creates a new binary input stream.  The stream is backed by a Mozilla
> platform stream that provides the underlying data, such as an
> `nsIFileInputStream`.
>+@param [backingStream] {file/stream}

I'm not sure how specific we're being with parameter types, but technically backingStream's type is an object, and specifically a stream object (not a file).  So it would be better to say {stream} or {object}.

>+<api name="ByteWriter">
>+@constructor
> Creates a new binary output stream.  The stream is backed by a Mozilla
> platform stream that actually writes out the data, such as an
> `nsIFileOutputStream`.
>+@param [backingStream] {file/stream}

Same here.

Comment 5

[Noelle Murata]

Attachment: with requested fixes per adw

Comment 6

[Drew Willcoxon :adw]

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

Yikes, sorry for forgetting about this Noelle.  (Feel free to ping me if I do it again! :)

>+<api name="ByteReader">
>+@constructor
> Creates a new binary input stream.  The stream is backed by a Mozilla
> platform stream that provides the underlying data, such as an
> `nsIFileInputStream`.
>+@param [backingStream] {stream}

backingStream is required, so no brackets here.

>+<api name="ByteWriter">
>+@constructor
> Creates a new binary output stream.  The stream is backed by a Mozilla
> platform stream that actually writes out the data, such as an
> `nsIFileOutputStream`.
>+@param [backingStream] {stream}

Same here.

>+<api name="write">
>+@method
> Writes to the stream.  If the stream is closed, an exception is thrown.
> *begin* and *end* are optional and control the portion of *str* that is output.
> If neither is specified, *str* is output in its entirety.  If only *begin* is
> specified, the suffix begining at that index is output.  If both are
> specified, the range <code>[*begin*, *end*)</code> is output.

Param names here are in asterisks, which makes them italic.  Is that consistent with apidocs, where italics mean optional?  Seems like the str param should not be in italics, since it's required.  Also, should param names in text like this be in a monospaced font, or what?

>+@param begin {number}
> *begin* is an optional argument specifying the index of *str* at which
> to start output.
> 
>+@param end {number}

begin and end are optional, so they should be in brackets.

Comment 7

[Noelle Murata]

Attachment: update byte streams

This this is a diff against -r b519a0848585

Comment 8

[Noelle Murata]

Attachment: update byte streams

please review!

Comment 9

[Noelle Murata]

Attachment: update byte streams

fixing the text inside of the bytewriter block.

Comment 10

[Drew Willcoxon :adw]

Comment on attachment 447399 [details] [diff] [review]
update byte streams

>-<code>ByteWriter.**write**(*str*, *begin*, *end*)</code>
>+<api name="write">
>+@method
>+Writes to the stream.  If the stream is closed, an exception is thrown.
>+*`begin`* and *`end`* are optional and control the portion of `str` that is output.
>+If neither is specified, `str` is output in its entirety.  If only *`begin`* is
>+specified, the suffix beginning at that index is output.  If both are
>+specified, the range *`[begin, end]`* is output.

[begin, end] -> [begin, end)

>+@param [begin] {number}
> *begin* is an optional argument specifying the index of *str* at which
> to start output.
> 
>+@param [end] {number}
> *end* is an optional argument specifying the index of *str* at which to end
> output.  The byte at index `end - 1` is the last byte output.

begin and end should also be monospaced like they are above, and str only monospaced like it is above.

As we discussed on IRC I'll make these small changes and push.  Thanks!

Comment 11

[Drew Willcoxon :adw]

http://hg.mozilla.org/labs/jetpack-sdk/rev/3dbc24feb708

Comment 12

[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".