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.

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

feat: rewrite docs generation #5309

Merged
merged 2 commits into from Oct 12, 2022
Merged

feat: rewrite docs generation #5309

merged 2 commits into from Oct 12, 2022

Conversation

lukekarrys
Copy link
Member

@lukekarrys lukekarrys commented Aug 16, 2022

TODO:

  • Test snapshots for all generated docs content
  • Gitignore only bundled deps

package.json Outdated Show resolved Hide resolved
@npm-cli-bot
Copy link
Collaborator

npm-cli-bot commented Aug 16, 2022

no statistically significant performance changes detected

timing results
app-large clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 46.294 ±1.82 25.841 ±0.58 23.590 ±0.24 26.804 ±1.07 4.051 ±0.10 4.087 ±0.22 3.219 ±0.10 17.082 ±0.52 3.211 ±0.03 4.326 ±0.07
#5309 49.043 ±3.53 27.003 ±0.20 30.165 ±10.80 28.351 ±3.28 3.981 ±0.00 4.034 ±0.10 3.158 ±0.06 16.734 ±0.04 3.162 ±0.05 4.702 ±0.13
app-medium clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 34.021 ±1.93 18.785 ±0.49 16.566 ±0.13 17.672 ±0.15 3.675 ±0.13 3.566 ±0.14 3.316 ±0.04 12.178 ±0.21 3.078 ±0.01 4.184 ±0.12
#5309 36.846 ±3.08 19.406 ±0.65 17.083 ±0.35 18.100 ±0.35 3.473 ±0.04 3.567 ±0.07 3.173 ±0.06 11.591 ±0.02 3.002 ±0.01 4.194 ±0.10

@lukekarrys
Copy link
Member Author

Since the man and html docs were generated, and we are no longer checking in the generated markdown docs, I did a full diff between the docs in 8.17.0 and in this branch BEFORE I removed all the extra placeholders. The full diff is below and it shows that we are now correctly replacing the versions, adding a simple synopsis for the npm command, and adding two copied man files.

Diff from 8.17.0.tgz

diff --git a/package/man/.DS_Store b/man/.DS_Store
index 63b47e9ed..73cb601a0 100644
Binary files a/package/man/.DS_Store and b/man/.DS_Store differ
diff --git a/package/man/man1/npm.1 b/man/man1/npm.1
index 04133f580..5f2b4a706 100644
--- a/package/man/man1/npm.1
+++ b/man/man1/npm.1
@@ -2,6 +2,12 @@
 .SH "NAME"
 \fBnpm\fR \- javascript package manager
 .SS Synopsis
+.P
+.RS 2
+.nf
+npm
+.fi
+.RE
 .SS Version
 .P
 8\.17\.0

diff --git a/man/man5/npm-global.5 b/man/man5/npm-global.5
new file mode 100644
index 000000000..b6efd3c88
--- /dev/null
+++ b/man/man5/npm-global.5
@@ -0,0 +1,226 @@

diff --git a/man/man5/npm-json.5 b/man/man5/npm-json.5
new file mode 100644
index 000000000..d4f972311
--- /dev/null
+++ b/man/man5/npm-json.5
@@ -0,0 +1,1296 @@

diff --git a/docs/content/.DS_Store b/docs/content/.DS_Store
new file mode 100644
index 000000000..22e638cd1
Binary files /dev/null and b/docs/content/.DS_Store differ

diff --git a/package/docs/content/commands/npm-ls.md b/docs/content/commands/npm-ls.md
index a7936fafc..64719f22c 100644
--- a/package/docs/content/commands/npm-ls.md
+++ b/docs/content/commands/npm-ls.md
@@ -36,7 +36,7 @@ packages will *also* show the paths to the specified packages.  For
 example, running \`npm ls promzard\` in npm's source tree will show:
 
 \`\`\`bash
-npm@@VERSION@ /path/to/npm
+npm@8.17.0 /path/to/npm
 └─┬ init-package-json@0.0.4
   └── promzard@0.1.5
 \`\`\`

diff --git a/package/docs/content/commands/npm.md b/docs/content/commands/npm.md
index 18a68d03c..013459dce 100644
--- a/package/docs/content/commands/npm.md
+++ b/docs/content/commands/npm.md
@@ -7,11 +7,21 @@ description: javascript package manager
 ### Synopsis
 
 <!-- AUTOGENERATED USAGE DESCRIPTIONS START -->
+<!-- automatically generated, do not edit manually -->
+<!-- see lib/commands/npm.js -->
+
+\`\`\`bash
+npm
+\`\`\`
+
+<!-- automatically generated, do not edit manually -->
+<!-- see lib/commands/npm.js -->
+
 <!-- AUTOGENERATED USAGE DESCRIPTIONS END -->
 
 ### Version
 
-@VERSION@
+8.17.0
 
 ### Description
 
diff --git a/package/docs/content/using-npm/package-spec.md b/docs/content/using-npm/package-spec.md
index 0d3741018..fd64131fb 100644
--- a/package/docs/content/using-npm/package-spec.md
+++ b/docs/content/using-npm/package-spec.md
@@ -4,7 +4,6 @@ section: 7
 description: Package name specifier
 ---
 
-
 ### Description
 
 Commands like \`npm install\` and the dependency sections in the
diff --git a/docs/output/.DS_Store b/docs/output/.DS_Store
new file mode 100644
index 000000000..f3ab3515d
Binary files /dev/null and b/docs/output/.DS_Store differ
diff --git a/package/docs/output/commands/npm.html b/docs/output/commands/npm.html
index 523629237..d0bbf8682 100644
--- a/package/docs/output/commands/npm.html
+++ b/docs/output/commands/npm.html
@@ -148,6 +148,12 @@ npm command-line interface
 <div id="_content"><h3 id="synopsis">Synopsis</h3>
 <!-- raw HTML omitted -->
 <!-- raw HTML omitted -->
+<!-- raw HTML omitted -->
+<pre lang="bash"><code>npm
+</code></pre>
+<!-- raw HTML omitted -->
+<!-- raw HTML omitted -->
+<!-- raw HTML omitted -->
 <h3 id="version">Version</h3>
 <p>8.17.0</p>
 <h3 id="description">Description</h3>

.github/workflows/ci.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@lukekarrys lukekarrys marked this pull request as draft August 16, 2022 23:11
@lukekarrys lukekarrys force-pushed the lk/docs-move branch 4 times, most recently from 04e7e0d to caf8c44 Compare August 17, 2022 04:07
@lukekarrys lukekarrys marked this pull request as ready for review August 17, 2022 04:15
@lukekarrys lukekarrys marked this pull request as draft August 17, 2022 04:26
@wraithgar
Copy link
Member

How are we to review changes in usage output now? It was very handy having them in source control to see for PRs. I suppose they still exist in the test snapshots for help/man? Do the full man pages exist in snapshots?

@lukekarrys
Copy link
Member Author

lukekarrys commented Aug 18, 2022

How are we to review changes in usage output now? It was very handy having them in source control to see for PRs. I suppose they still exist in the test snapshots for help/man? Do the full man pages exist in snapshots?

That's a good point. I just checked and we have snapshots for config's describeAll() and shorthands. I'm going to add snapshots that run for all commands to output their usage and params.

Also docs generation will fail if anything couldn't be replaced, so if something isn't being generated properly we will learn about it during CI (or if you run npm run docs locally).

@wraithgar wraithgar mentioned this pull request Aug 18, 2022
lukekarrys pushed a commit that referenced this pull request Aug 22, 2022
@fritzy fritzy force-pushed the latest branch 2 times, most recently from 3037d35 to f3b0c43 Compare September 14, 2022 23:09
@lukekarrys lukekarrys force-pushed the lk/docs-move branch 2 times, most recently from a17dfcb to 2751b41 Compare October 5, 2022 19:44
docs/lib/transform.js Fixed Show fixed Hide fixed
@lukekarrys lukekarrys force-pushed the lk/docs-move branch 2 times, most recently from 5ed2bcc to 1bdb836 Compare October 6, 2022 05:23
docs/lib/index.js Fixed Show fixed Hide fixed
@lukekarrys lukekarrys marked this pull request as ready for review October 6, 2022 07:12
@lukekarrys
Copy link
Member Author

lukekarrys commented Oct 6, 2022

There are a lot of commits here with bad messages, but this PR should be squashed. So this is ready for review 😎

Squashed this to add some deps commits that we can update to in the docs workspace, so this should now be rebased.

@lukekarrys
Copy link
Member Author

lukekarrys commented Oct 6, 2022

High level overview of the changes here:

  • The source for the docs content has moved from docs/content/ to docs/lib/content/. The generated markdown is still written to docs/content/ but that directory is now ignored from git.
  • All generated content sections of the docs have been removed and replaced with single placeholder html comments such as <!-- AUTOGENERATED CONFIG DESCRIPTIONS -->
  • Placeholders are replaced with generated content only as part of the prepack step, so generated markdown is no longer checked in to source and all docs related make commands have been removed
  • All docs (and docs related) snapshots have been moved to a single test file that outputs command usage and formats it with functions imported from docs/lib/index.js. So tests will fail if docs content changes until npm run snap is run.

@lukekarrys lukekarrys requested review from wraithgar and a team October 6, 2022 16:02
@lukekarrys lukekarrys force-pushed the lk/docs-move branch 2 times, most recently from ab0845d to ca707ca Compare October 11, 2022 18:08
High level overview of the changes here:

- The source for the docs content has moved from `docs/content/` to
  `docs/lib/content/`. The generated markdown is still written to
  `docs/content/` but that directory is now ignored from git.
- All generated content sections of the docs have been removed and
  replaced with single placeholder html comments such as `<!--
  AUTOGENERATED CONFIG DESCRIPTIONS -->`
- Placeholders are replaced with generated content only as part of the
  `prepack` step, so generated markdown is no longer checked in to
  source and all docs related `make` commands have been removed
- All docs (and docs related) snapshots have been moved to a single test
  file that outputs command usage and formats it with functions imported
  from `docs/lib/index.js`. So tests will fail if docs content changes
  until `npm run snap` is run.
@fritzy fritzy merged commit 6c5c2a8 into latest Oct 12, 2022
@fritzy fritzy deleted the lk/docs-move branch October 12, 2022 20:04
lukekarrys added a commit that referenced this pull request Oct 13, 2022
After #5309 moved docs dependencies to proudction deps, we started
failing our daily audit CI check. Currently these deps are production
so they are available when we run `pack`, but they don't need to be
audited since they are never present in our published tarball.

This change runs `audit` on the root CLI and all workspaces within the
`workspaces/` directory, which are the only production workspaces.
fritzy pushed a commit that referenced this pull request Oct 13, 2022
After #5309 moved docs dependencies to proudction deps, we started
failing our daily audit CI check. Currently these deps are production
so they are available when we run `pack`, but they don't need to be
audited since they are never present in our published tarball.

This change runs `audit` on the root CLI and all workspaces within the
`workspaces/` directory, which are the only production workspaces.
lukekarrys added a commit that referenced this pull request Oct 27, 2022
This moves all the dependencies of the `docs/` workspace to dev deps. I
had originally moved them out of devDeps as part of #5309, but this
seems to go against the grain of other tooling. We would have to special
case both `audit` and `licensee` to run on a custom subset of our
dependency tree.

Maybe one day when all tools can be piped the output of an `npm query`
this will be possible. It would be nice if we could only audit
dependencies that are bundled as part of the CLI.

But for now it's easier to move these to devDeps and then reinstall only
the docs workspace after pruning during the publish step.
lukekarrys added a commit that referenced this pull request Nov 1, 2022
This moves all the dependencies of the `docs/` workspace to dev deps. I
had originally moved them out of devDeps as part of #5309, but this
seems to go against the grain of other tooling. We would have to special
case both `audit` and `licensee` to run on a custom subset of our
dependency tree.

Maybe one day when all tools can be piped the output of an `npm query`
this will be possible. It would be nice if we could only audit
dependencies that are bundled as part of the CLI.

But for now it's easier to move these to devDeps and then reinstall only
the docs workspace after pruning during the publish step.
wraithgar pushed a commit that referenced this pull request Nov 1, 2022
This moves all the dependencies of the `docs/` workspace to dev deps. I
had originally moved them out of devDeps as part of #5309, but this
seems to go against the grain of other tooling. We would have to special
case both `audit` and `licensee` to run on a custom subset of our
dependency tree.

Maybe one day when all tools can be piped the output of an `npm query`
this will be possible. It would be nice if we could only audit
dependencies that are bundled as part of the CLI.

But for now it's easier to move these to devDeps and then reinstall only
the docs workspace after pruning during the publish step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants