Closed jugglinmike closed 5 years ago
From @jugglinmike (originally posted in gh-10):
@jgraham the final commit on this branch is what I'm referring to in this pull request's description. What do you think about maintaining code like this in wptrunner?
From @jgraham (originally posted in gh-10):
That change looks fine; not sure why it's not a PR against upstream though, since reviewing a big-bang commit with all these changes in won't end up with meaningful review. I'm not sure how wptrunner is related though; where wptrunner is exposing something different to the
wpt
subcommands that wrap it, the wpt variants are the ones that should be documented.
Thanks, @jgraham
That change looks fine; not sure why it's not a PR against upstream though, since reviewing a big-bang commit with all these changes in won't end up with meaningful review.
Here's what I was thinking when I created this fork
We recognize a bit of tension between the different ways we could conduct this work and submit the result. The preferred git workflow includes many small, targeted patches. However, restructuring a large body of text in a holistic way will tend towards more wide-spanning change sets. Taking a cue from similar work @gsnedders conducted back in 2017, we're going to collaborate on a restructuring from a fork of WPT.
Ideally, substantive review like your own will take place in this repository so that there aren't any objections to merging the interrelated work atomically.
I'm not sure how wptrunner is related though; where wptrunner is exposing something different to the
wpt
subcommands that wrap it, the wpt variants are the ones that should be documented.
My bad: in my experience, wpt
and wptrunner
have been equivalent, so I use the terms interchangeably. I think we're in agreement conceptually, though, since as you say, this patch actually extends code in tools/wpt/
, not tools/wptrunner/
.
Thanks for the review. @zcorpan! Rebased and merged to master
at commit 2d165248ae861bf1cdcf9bed1d4467ea1629cdd8
@jugglinmike I thought I have commented but apparently no...
This is really awesome and useful! CLI flags have long been a pain point when I introduce people to WPT tools.
Thanks for saying so, @Hexcles. I've since followed up by refactoring the linting source in WPT, so we're one step closer.
Command-line usage information was previously only available to users who had successfully installed wptrunner. Consuming the information in that form (e.g. navigating between sub-commands, scrolling through pages of content and searching for keywords) also required some proficiency with the command-line.
Extend the documentation build process to include this information in the project website. This makes it easier to consume for those with limited experience using the command-line, and in particular, it adds the content to the data indexed by the website's "search" feature.
Because this content is generated from the state of the interface, it avoids the need to maintain documentation separately.
This is implemented via a new code path that eagerly load wptrunner's complete command-line interface. The new
create_complete_parser
is consumed by thesphinx-argparse
module to produce the rendered documentation.This is one way we can automatically include documentation for the
wpt
command-line interface in the documentation website.The wptrunner CLI is built lazily based on the subcommand being executed. Even then, the task of argument parsing is split across unrelated
argparse
instances. These two details resist documentation generation (at least in the terms expected by thesphinx-argparse
module).Facilitating this work required an artificial code path. I'm sensitive to creating branches that aren't used by contributors, but given the small size of the new function, I think the documentation may be valuable enough to justify the maintenance burden.
Here's what the page looks like by default. (We should certainly tweak the appearance, but I'd like to verify that the high-level approach is acceptable before diving in to that.)
Like any other content on the Sphinx-powered site, it is indexed by the client-side "search" feature.
And the user's query is highlighted on the page itself
Depends on gh-7.