Bring the doctool up to date on v4.x #7668
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
addaleax
added
doc
nodejs-github-bot
added
http
|
WOWOW code forensics. So this biggest question for me in all this is if we want to include 294d370 as it is semver-minor /cc @nodejs/lts Perhaps we should talk about this big backport briefly on the call on Wednesday (adding label) |
I was assuming that, yes. It’s definitely non-essential here, but I saw an opportunity to do something that basically just made sense. |
MylesBorins
force-pushed
the
v4.x-staging
branch
from
808fb1f
to
9079d87
Compare
addaleax
force-pushed
the
backport-doctool-stuff
branch
from
55dce5c
to
541587f
Compare
|
Rebased against the current v4.x-staging, CI: https://ci.nodejs.org/job/node-test-commit/4070/ |
MylesBorins
force-pushed
the
v4.x-staging
branch
from
08f0120
to
1e66668
Compare
addaleax
force-pushed
the
backport-doctool-stuff
branch
from
541587f
to
8374db1
Compare
MylesBorins
force-pushed
the
v4.x-staging
branch
from
fcdb93d
to
27222f4
Compare
|
@addaleax I'm going to request another rebase here and to drop 294d370. I don't think that is a valid candidate for LTS You may find a spurious |
addaleax
force-pushed
the
backport-doctool-stuff
branch
from
8374db1
to
974e574
Compare
|
Rebased & removed the HTTP commit. |
Add a hack js-yaml module to the doctool dependencies that simply loads the one that’s included with eslint. This helps avoiding to check in the whole dependency tree into the core repo. PR-URL: nodejs#6495 Reviewed-By: Robert Jefe Lindstaedt <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
This commit introduces the Documentation YAML metadata concept to the documentation. - Parses added or Added for HTML - Parses all data for JSON Ref: nodejs#3867 Ref: nodejs#3713 Ref: nodejs#6470 PR-URL: nodejs#6495 Reviewed-By: Robert Jefe Lindstaedt <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Add checks that make sure the doctool parses metadata correctly. PR-URL: nodejs#6495 Reviewed-By: Robert Jefe Lindstaedt <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Allow multiple `added:` version entries, since semver-minors can trickle down to previous major versions, and thus features may have been added in multiple versions. Also include `deprecated:` entries and apply the same logic to them for consistency. Stylize the added HTML as `Added in:` and `Deprecated since:`. PR-URL: nodejs#6495 Reviewed-By: Robert Jefe Lindstaedt <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Update module marked. Customize renderer to remove id from heading. PR-URL: nodejs#6396 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
There has been occasional nits for spacing in object literals in PRs but
the project does not lint for it and it is not always handled
consistently in the existing code, even on adjacent lines of a file.
This change enables a linting rule requiring no space between the key
and the colon, and requiring at least one space (but allowing for more
so property values can be lined up if desired) between the colon and the
value. This appears to be the most common style used in the current code
base.
Example code the complies with lint rule:
myObj = { foo: 'bar' };
Examples that do not comply with the lint rule:
myObj = { foo : 'bar' };
myObj = { foo:'bar' };
PR-URL: nodejs#6592
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Brian White <[email protected]>
PR-URL: nodejs#6652 Reviewed-By: Anna Henningsen <[email protected]>
Use the overload of `v8::Function::NewInstance()` that returns a `v8::MaybeLocal<v8::Object>`. The overloads that return a simple `v8::Local<v8::Object>` are deprecated. PR-URL: nodejs#6652 Reviewed-By: Anna Henningsen <[email protected]>
* Make the 'extract embedded addons in the documentations' step a normal prerequisite. As an order-only prerequisite, it's sometimes skipped when it shouldn't be. * Make `tools/doc/addon-verify.js` a dependency of that step. Changes to that file should result in a rebuild. PR-URL: nodejs#6652 Reviewed-By: Anna Henningsen <[email protected]>
Introduced in commit 3f69ea5 ("tools: update marked dependency"), it stopped the embedded addons in the documentation from getting built. PR-URL: nodejs#6652 Reviewed-By: Ben Noordhuis <[email protected]>
Allows building just docs using existing Node instead of building Node first. PR-URL: nodejs#3888 Reviewed-By: Chris Dickinson <[email protected]> Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Myles Borins <[email protected]>
101dd1e introduced a regression in the doctool. This commit reverts the changes that were made to the function signature of the various doctool functions while maintaining support for passing in specific node versions. Refs: nodejs@101dd1e PR-URL: nodejs#6680 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Robert Lindstaedt <[email protected]>
Adjust style in doctool tests to conform with predominant style of the rest of the project. The biggest changes are: * Replace string concatenation with `path.join()` * Remove unnecessary quotes from property names PR-URL: nodejs#6719 Reviewed-By: James M Snell <[email protected]>
These signatures were originally converted to opts hashes in nodejs#3888. That change was misinterpreted as the intrinsic cause of a test failure and reverted in nodejs#6680. PR-URL: nodejs#6690 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
Previously, output files which were created using includes (notably, the single-page all.html) had basically broken internal links all over the place because references like `errors.html#errors_class_error` are being used, yet `id` attributes were generated that looked like `all_class_error`. This PR adds generation of comments from the include preprocessor that indicate from which file the current markdown bits come and lets the HTML output generation take advantage of that so that more appropriate `id` attributes can be generated. PR-URL: nodejs#6943 Reviewed-By: Robert Jefe Lindstaedt <[email protected]> Reviewed-By: Daniel Wang <[email protected]>
Ref: nodejs#6578 PR-URL: nodejs#6864 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Add a `REPLACEME` tag that should be used when introducing docs for new features, so that they can be updated when releases are made. Ref: nodejs#6578 PR-URL: nodejs#6864 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Skip the doctool tests when js-yaml, which is currently `require()`d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs. Fixes: nodejs#7201 Ref: nodejs#6495 PR-URL: nodejs#7218 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
After the nodejs#3888 it was not possible to "make doc-only" in some situations. This now fallsback to any installed node version and throws "node not found" in error case. Ref: nodejs#3888 PR-URL: nodejs#6906 Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
addaleax
force-pushed
the
backport-doctool-stuff
branch
from
974e574
to
76c7918
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport:
tools: parse documentation metadata (take 2)tools: update marked dependencyhttp: 451 status code "Unavailable For Legal Reasons"– semver-minor HTTP status code addition. Completely unrelated to the rest of the changes except to make the subsequent commit land cleaner… maybe /cc @nodejs/http @nodejs/lts for a green light on this, but I’m fairly certain it should be okay.tools: lint for object literal spacing– Again, only to make things land cleaner; plus extra whitespace changes to be squashed that make the linter happy with this change on v4.tools: fix tools/doc/addon-verify.js regressionbuild: add Makedoc-onlytarget– @thealphanerd You explicitly stated here that you would like to opt this out of LTS, presumably because of the regression fixed in tools: fix regression in doctool #6680/tools: restore change of doc building function signatures to opts hashes #6690… would you be okay with including these together?tools: fix regression in doctooltest: refactor doctool teststools: restore change of doc building function signatures to opts hashes(aka the failing part of Add task for building docs only, using existing Node #3888 done right)tools: make sure doctool anchors respect@include``build: add REPLACEME tag for version info in docstest: skip doctool tests when js-yaml is missingdoc: make doc-only -> fallback to user binaryI’m sorry.