Update cfx docs for --binary-args and --no-run options

RESOLVED FIXED

Status

Add-on SDK
Documentation
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: wbamberg, Assigned: wbamberg)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
See bug 646345 and bug 646343.
(Assignee)

Comment 1

7 years ago
Created attachment 531147 [details] [diff] [review]
Document the --binary-args and --no-run options

The Internal/Experimental classification follows that given by cfx --help. But while I understand the motivation for --no-run being experimental, I'm a bit surprised --binary-args is Internal. It seems to uncontroversially and generally useful enough to be Supported.
Attachment #531147 - Flags: review?(warner-bugzilla)
Comment on attachment 531147 [details] [diff] [review]
Document the --binary-args and --no-run options

Looks good in general. A few thoughts:

 * the <pre> block with the firebox-bin command is confusing: the /-Tmp-/ portion is split over multiple lines, and having each line start in col0 makes it feel like you're supposed to type 5 separate lines instead of just one. I'd sugggest adding a one-space indent to all but the first line. Also, maybe <tt> is better than <pre>, and remove the hard newlines. (in general, getting long command lines into documentation without confusing wrapping or excessive scrolling is a hard problem, not sure what to do here).
 * duplicating the docs (for both 'cfx run' and 'cfx test') is unfortunate. Could one section say "see X for details" and reference the other?
 * for -binary-args, I'd explain why "binary" is in the name (e.g. "pass extra arguments to the binary being executed, e.g. firefox). I'd also have an example of passing multiple arguments (space-separated? needs quotes? use multiple -binary-args args?), since the argname itself is plural.

But I'm happy with the changes as-is. r+ from me with or without those suggestions.
Attachment #531147 - Flags: review?(warner-bugzilla) → review+
(Assignee)

Comment 3

7 years ago
Thanks for the feedback Brian. Agree about <pre> and --binary-args.

>  * duplicating the docs (for both 'cfx run' and 'cfx test') is unfortunate.
> Could one section say "see X for details" and reference the other?

I went back and forth a bit with the structure of this doc the last time I revised it. Yes, repetition is unfortunate, but as a reference doc the basic usage is to see everything about, say, cfx run, rather than to read the doc start to finish. 

That being so I think avoiding repetition only helps the maintainer, not the reader, and that it's unhelpful to present the command followed by a list of incomprehensible options and have to cross-reference to see what they all are (even if that's done using links). I think it's nice to be able to see everything about the command in one place, and that the tabular format is a much more readable way to present this than a list of links.

But I'm happy to reassess this :-).
(Assignee)

Comment 4

7 years ago
Fixed by: https://github.com/mozilla/addon-sdk/commit/0fc8c85a13a88eb37e9bf2f053593d656b94cabd
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(In reply to comment #1)
> The Internal/Experimental classification follows that given by cfx --help.
> But while I understand the motivation for --no-run being experimental, I'm a
> bit surprised --binary-args is Internal. It seems to uncontroversially and
> generally useful enough to be Supported.

Indeed, my bad for not catching that in review.

Also, it might be helpful to mention in the docs that --no-run creates a profile directory that it doesn't clean up, although it's not a tragedy to omit that detail, since the profile directory will be created in the temporary directory, and thus it'll get deleted by the OS eventually.
(In reply to comment #5)
> (In reply to comment #1)
> > The Internal/Experimental classification follows that given by cfx --help.
> > But while I understand the motivation for --no-run being experimental, I'm a
> > bit surprised --binary-args is Internal. It seems to uncontroversially and
> > generally useful enough to be Supported.
> 
> Indeed, my bad for not catching that in review.

Filed as bug 656542.
You need to log in before you can comment on or make changes to this bug.