Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use JSDoc for documentation #31440

Merged
merged 21 commits into from
Nov 23, 2021
Merged

Use JSDoc for documentation #31440

merged 21 commits into from
Nov 23, 2021

Conversation

jgraham
Copy link
Contributor

@jgraham jgraham commented Oct 29, 2021 •
edited
Loading

The goal here is to address some complaints around the quality of the API documentation, and to make the docs easier to maintain going forward. To that end there are two big changes:

  • The addition of sphinx-js to enable adding JSDoc strings directly into the docs.
  • Conversion of the testharness and testdriver docs to use the JSDoc rather than repeating all the documentation in the markdown files as well as in the source.

There are also some incidential changes:

  • The readdition of a Dockerfile for generating docs. The wpt build-docs command is updated to optionally use this docker image, avoiding the need to install the js components locally.
  • The addition of sphinx-autoreload so that it's possible to work on the docs using a local server and have the output regenerated when the inputs change.

@jgraham jgraham requested a review from jugglinmike October 29, 2021 13:45
@jgraham jgraham force-pushed the docs branch 3 times, most recently from fdbe5ce to a17e3c9 Compare October 29, 2021 14:07
@jgraham jgraham force-pushed the docs branch 2 times, most recently from 4d8caee to 7230491 Compare November 5, 2021 15:47
@jgraham
Copy link
Contributor Author

jgraham commented Nov 5, 2021

Rendered changes (current state):
https://jgraham.github.io/web-platform-tests/writing-tests/testharness-api.html
https://jgraham.github.io/web-platform-tests/writing-tests/testdriver.html

@jgraham jgraham closed this Nov 5, 2021
@jgraham jgraham reopened this Nov 5, 2021
@jgraham jgraham requested review from foolip and gsnedders November 5, 2021 22:15
Ms2ger
Ms2ger reviewed Nov 8, 2021
View reviewed changes
docs/frontend.py Outdated Show resolved Hide resolved
docs/frontend.py Outdated Show resolved Hide resolved
docs/frontend.py Outdated Show resolved Hide resolved
docs/frontend.py Outdated Show resolved Hide resolved
docs/frontend.py Outdated Show resolved Hide resolved
docs/frontend.py Show resolved Hide resolved
@jgraham jgraham changed the title Update docs generation Use JSDoc for documentation Nov 8, 2021
@jgraham jgraham force-pushed the docs branch 3 times, most recently from 26a9df4 to 1f13427 Compare November 8, 2021 17:53
@jgraham jgraham marked this pull request as ready for review November 8, 2021 19:05
@jgraham jgraham requested a review from Ms2ger November 8, 2021 19:10
@sideshowbarker sideshowbarker removed their request for review November 9, 2021 07:29
@foolip
Copy link
Member

foolip commented Nov 16, 2021

@jgraham there are conflicts to resolve here. I guess that this is ready for review apart from that though?

@jgraham
Copy link
Contributor Author

jgraham commented Nov 16, 2021

Yes, this is ready for review.

jgraham and others added 12 commits November 22, 2021 16:38
@jgraham
Add support for running a docs server with auto-reload.
ab45a37 
This adds a `--serve` option to `wpt build-docs` which will run a
local server. sphinx-autobuild is used so that the docs are
automatically rebuilt when relevant files change.
@jgraham
Tests can't have a status of ERROR, so don't try to set that in asser…
3f8a84e 
…t_wrapper
@jgraham
Rewrite testharness API docs
4889bbb 
This extracts more of the documentation from the JSDoc comments in the
file itself and reorganises things a little so the documentation is
easier to navigate.
@jgraham
Rewrite the testdriver docs.
28c8a3f 
This extracts most of the documentation from the JSDoc comments rather
than maintaining it in two places.

Due to the limitations of sphinx-js, it seems like every command needs
to be included one-by-one in the docs. However that has the advantage
that we can give them some orgnaisation rather than just providing a
flat list of commands.
@jgraham
Use repo from environment rather than hardcoding
1fa7c6e
@jgraham
Add ToC to tutorial
5a9bf41
@foolip @jgraham
Update docs/writing-tests/testharness-api.md
3c90f48
@foolip @jgraham
Update resources/testharness.js
95ed759
@jgraham
Use package.json to specify node deps
aa3ae2f
@jgraham
Sort docs requirements
ef3f6f5
@jgraham
Update links in testharness.js
20ac6a8
@jgraham
Update step_func documentation
aac8664
foolip
foolip approved these changes Nov 22, 2021
View reviewed changes
docs/Dockerfile

COPY ./package.json ./
RUN npm install .
ENV PATH=/app/node_modules/.bin:$PATH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be necessary, but maybe npx could be used, at least if invoked from /app/.

docs/package.json Outdated Show resolved Hide resolved
resources/testharness.js Outdated Show resolved Hide resolved
@jgraham jgraham force-pushed the docs branch 7 times, most recently from ae77d98 to e212f94 Compare November 23, 2021 10:46
jgraham and others added 2 commits November 23, 2021 12:40
@jgraham
Run npm install in the docs directory in CI
43a887a 
Trying to run it in the parent seems to not work in npm 6.x
@jgraham @foolip
Apply suggestions from code review
a237be8 
Co-authored-by: Philip Jägenstedt <[email protected]>
@jgraham
Fix documentation of Test.force_timeout
3af219c 
I couldn't get sphinx-js to work with @name or @function, so it seems
to be easier just to make an actual wrapper function so the docs render correctly.
@jgraham jgraham merged commit 2c2855c into master Nov 23, 2021
@jgraham jgraham deleted the docs branch November 23, 2021 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@foolip foolip foolip approved these changes

@jugglinmike jugglinmike Awaiting requested review from jugglinmike

@gsnedders gsnedders Awaiting requested review from gsnedders

@Ms2ger Ms2ger Awaiting requested review from Ms2ger

Assignees

@jgraham jgraham

Labels
ci docs GitHub Actions infra testdriver.js testharness.js
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jgraham @foolip @Ms2ger @wpt-pr-bot