Andreas Tolfsen ❲:ato❳ Assignee
	Description • 6 years ago

Sphinx now supports embedding jsdoc documentation, and instead of
statically generating testing/marionette/doc/api manually we can
hook into that and have the documentation automatically generated
on TC, and published directly to http://firefox-source-docs.mozilla.org/.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 1 • 6 years ago

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2468a099357963f9254f869bca5db42a29de4f38

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 2 • 6 years ago

Hurray! Does it mean we finally get rid of the generated HTML files in-tree? That is wonderful!

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 15 • 6 years ago

The attached patches remove the statically generated documentation
and sets up the integration with sphinx-js.

They also make a start at converting some HTML-flavoured JSDocs
into RST, which apparently is required if we are to publish
with Sphinx.  I don’t particularly like the extra churn this
causes, but I think there is inherent value in being published on
http://firefox-source-docs.mozilla.org/ that perhaps makes it worth it.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 16 • 6 years ago
mozreview-review

Comment on attachment 8953952 [details]
Bug 1441019 - Remove pre-generated Marionette API docs.

https://reviewboard.mozilla.org/r/223094/#review229194

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 17 • 6 years ago
mozreview-review

Comment on attachment 8953953 [details]
Bug 1441019 - Remove jsdoc config.

https://reviewboard.mozilla.org/r/223096/#review229196

	Nick Alexander :nalexander [he/him]
	Comment 18 • 6 years ago
mozreview-review

Comment on attachment 8953957 [details]
Bug 1441019 - Add testing/marionette to js_source_path.

https://reviewboard.mozilla.org/r/223104/#review229198

Sure.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 19 • 6 years ago
mozreview-review

Comment on attachment 8953954 [details]
Bug 1441019 - Remove "./mach marionette doc" command.

https://reviewboard.mozilla.org/r/223098/#review229200

::: commit-message-c16a6:3
(Diff revision 1)
> +Bug 1441019 - Remove "./mach marionette test" command. r?whimboo
> +
> +This removes the "./mach marionette test" command in favour of the

You mean `./mach marionette doc` here, right?

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 20 • 6 years ago
mozreview-review

Comment on attachment 8953955 [details]
Bug 1441019 - Remove duplicate namespace declaration.

https://reviewboard.mozilla.org/r/223100/#review229202

Maybe mention that this is for event.js.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 21 • 6 years ago
mozreview-review

Comment on attachment 8953956 [details]
Bug 1441019 - Differentiate WorkerDebuggerTransport implementations.

https://reviewboard.mozilla.org/r/223102/#review229206

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 22 • 6 years ago
mozreview-review

Comment on attachment 8953958 [details]
Bug 1441019 - Add jsautodoc indices.

https://reviewboard.mozilla.org/r/223106/#review229210

Please fill-in at least the basics for cookie.rst.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 23 • 6 years ago
mozreview-review

Comment on attachment 8953959 [details]
Bug 1441019 - Migrate cookie module docs to RST.

https://reviewboard.mozilla.org/r/223108/#review229212

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 24 • 6 years ago
mozreview-review

Comment on attachment 8953960 [details]
Bug 1441019 - Document Marionette error module.

https://reviewboard.mozilla.org/r/223110/#review229218

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 25 • 6 years ago
mozreview-review

Comment on attachment 8953961 [details]
Bug 1441019 - Migrate dom module docs to RST.

https://reviewboard.mozilla.org/r/223112/#review229220

::: testing/marionette/doc/internals/dom.rst
(Diff revision 1)
>  dom module
>  ==========
>  
> -ContentEventObserverService
> ----------------------------

Better to kill those lines in the initial patch.

::: testing/marionette/dom.js:13
(Diff revision 1)
>    "ContentEventObserverService",
>    "WebElementEventTarget",
>  ];
>  
>  /**
> - * The {@link EventTarget} for web elements can be used to observe DOM
> + * The ``EventTarget`` for web elements can be used to observe DOM

Does that render as link, or do we have to keep something like :js:property:`EventTarget`?

::: testing/marionette/dom.js:22
(Diff revision 1)
> - * to listen for top-level <code>window</code> global events.
> + * to listen for top-level ``window`` global events.
>   *
> - * It needs to be backed by a {@link ContentEventObserverService} in a
> - * content frame script.
> + * It needs to be backed by a :js:class:`ContentEventObserverService`
> + * in a content frame script.
>   *
> - * Usage:
> + * Usage::

Why a double-colon?

::: testing/marionette/dom.js
(Diff revision 1)
> - * It needs to be backed by a {@link ContentEventObserverService} in a
> - * content frame script.
> + * It needs to be backed by a :js:class:`ContentEventObserverService`
> + * in a content frame script.
>   *
> - * Usage:
> + * Usage::
>   *
> - * <pre><code>

So we don't have to instruct sphinx-js to render something as code?

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 26 • 6 years ago
mozreview-review-reply

Comment on attachment 8953961 [details]
Bug 1441019 - Migrate dom module docs to RST.

https://reviewboard.mozilla.org/r/223112/#review229220

> Does that render as link, or do we have to keep something like :js:property:`EventTarget`?

It will just render as monospace.  The JSDoc link from before never
lead anywhere because the EventTarget WebIDL isn’t defined.

> Why a double-colon?

This is RST syntax for pre-formatted monospace stuff, like code
examples.

> So we don't have to instruct sphinx-js to render something as code?

Implied by the double colon.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 27 • 6 years ago
mozreview-review

Comment on attachment 8953962 [details]
Bug 1441019 - Migrate sync module docs to RST.

https://reviewboard.mozilla.org/r/223114/#review229224

::: testing/marionette/doc/internals/sync.rst:4
(Diff revision 1)
>  sync module
>  ===========
>  
> +Provides an assortment of synchronisation primitives.

Best to already add this to the initial patch.

::: testing/marionette/sync.js
(Diff revision 1)
> + * The passed value to ``reject`` will also be returned to the caller
> + * once the wait has expired.
> + *
> + * Usage::
>   *
> - * <pre><code>

Same question regarding code examples as in the other commit.

::: testing/marionette/sync.js:118
(Diff revision 1)
>  /**
> - * The <code>TimedPromise</code> object represents the timed, eventual
> - * completion (or failure) of an asynchronous operation, and its
> + * Represents the timed, eventual completion (or failure) of an
> + * asynchronous operation, and its resulting value.
> - * resulting value.
>   *
> - * In contrast to a regular {@link Promise}, it times out after
> + * In contrast to a regular promise, it times out after ``timeout``.

`Promise` please.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 28 • 6 years ago
mozreview-review

Comment on attachment 8953963 [details]
Bug 1441019 - Add overviews and links to new API docs.

https://reviewboard.mozilla.org/r/223116/#review229256

::: testing/marionette/doc/index.rst:54
(Diff revision 1)
>     `Contributing to Marionette`_
>     NewContributors.md
>     Debugging.md
>     PythonTests.md
>     SeleniumAtoms.md
> +   internals/index

Internals doesn't say that much, but not sure if there is something better.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 29 • 6 years ago
mozreview-review-reply

Comment on attachment 8953962 [details]
Bug 1441019 - Migrate sync module docs to RST.

https://reviewboard.mozilla.org/r/223114/#review229224

> Same question regarding code examples as in the other commit.

Double colon will start a code section.

	Gregory Szorc [:gps]
	Comment 42 • 6 years ago
mozreview-review

Comment on attachment 8953957 [details]
Bug 1441019 - Add testing/marionette to js_source_path.

https://reviewboard.mozilla.org/r/223104/#review229304

We should really get JS docs support in moz.build files...

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 43 • 6 years ago
mozreview-review

Comment on attachment 8953962 [details]
Bug 1441019 - Migrate sync module docs to RST.

https://reviewboard.mozilla.org/r/223114/#review229462

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 44 • 6 years ago
mozreview-review

Comment on attachment 8953961 [details]
Bug 1441019 - Migrate dom module docs to RST.

https://reviewboard.mozilla.org/r/223112/#review229464

	Pulsebot
	Comment 45 • 6 years ago

Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/64aac0ce028b
Remove pre-generated Marionette API docs. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/2148942fab87
Remove jsdoc config. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/e490cf5bb284
Remove "./mach marionette doc" command. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/7ec2b7ddb765
Remove duplicate namespace declaration. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/bcf02942d86e
Differentiate WorkerDebuggerTransport implementations. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/bc74d07ad0da
Add testing/marionette to js_source_path. r=gps,nalexander
https://hg.mozilla.org/integration/autoland/rev/af130eb9481f
Add jsautodoc indices. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/0995940608cf
Migrate cookie module docs to RST. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/266b7514fc1f
Document Marionette error module. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/ffbc1e20455e
Migrate dom module docs to RST. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/00b77f68b9fb
Migrate sync module docs to RST. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/8e612e57fc49
Add overviews and links to new API docs. r=whimboo

	Andreea Pavel [:apavel]
	Comment 46 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/64aac0ce028b
https://hg.mozilla.org/mozilla-central/rev/2148942fab87
https://hg.mozilla.org/mozilla-central/rev/e490cf5bb284
https://hg.mozilla.org/mozilla-central/rev/7ec2b7ddb765
https://hg.mozilla.org/mozilla-central/rev/bcf02942d86e
https://hg.mozilla.org/mozilla-central/rev/bc74d07ad0da
https://hg.mozilla.org/mozilla-central/rev/af130eb9481f
https://hg.mozilla.org/mozilla-central/rev/0995940608cf
https://hg.mozilla.org/mozilla-central/rev/266b7514fc1f
https://hg.mozilla.org/mozilla-central/rev/ffbc1e20455e
https://hg.mozilla.org/mozilla-central/rev/00b77f68b9fb
https://hg.mozilla.org/mozilla-central/rev/8e612e57fc49