-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
tools: parse documentation metadata (take 2) #6495
Conversation
I'm really not sure we should be checking this into core; could it be part of the website tooling instead? |
Unsure about which part being in core? Just the tool or the metadata? Are
|
Just to be clear, this is not adding anything radically new to core. It’s adding a feature to the already existing doctool; and as far as that is concerned, I’d be +1 on moving that out of the core repo, but that seems like a completely different discussion to me.
|
Oh, and if you’re talking about the diff size: I get that. Not so excited about it either, but that’s mostly dependencies and people were fine with it in the original PR, so I just went with it. |
Thank you @addaleax! I've never been that happy with all the node_modules we check into the repo (for eslint, doc, npm, etc). I'd love to find an effective way of moving those out. |
Very much +1 to that, @jasnell. I mean, eslint brings an impressive 12MB copy of lodash into the repo, and this PR in its current state would add another (older, therefore smaller) copy. I think having node_modules/ for npm is unavoidable without having some sort of chicken-egg-problem, but how about installing the deps from |
@addaleax If you haven't done so already, you can use https://www.npmjs.com/package/dmn to slightly reduce the number of files in |
The metadata is fine, it's hidden in an html comment anyways. The toolchain is large & will likely change significantly over time, imo. Also, this isn't as critical as the base docs parsing, nor our bundling of npm and eslint. |
@Trott Thanks for the pointer… that works, but it doesn’t seem to help a lot. I’ll try to edit the first commit with updated dependency versions, that seems to help much more. |
cc43f2a
to
2715e37
Compare
Updated this PR with a hack to load js-yaml from the eslint dependency tree. Definitely not a perfect solution, but it works. @Fishrock123 Is this something that would address your concerns? |
@addaleax ... personally, I certainly wouldn't mind having to do a |
@jasnell Yes, but let’s save that for another PR. ;-) |
@jasnell Is this something we actually need to install in CI though? If we saved this for |
Not really, that's actually more concerning imo. I think we should explore having an npm install for this additional info. :) edit: p.s. I'm in favor of this change but really would like to not be checking in even more npm module to the git source. |
The doctool is being tested in CI, as far as I can tell. But if you want this whole thing optional anyway, then the tests would probably have to detect the availability, too.
Just something to keep in mind: Adding copies of files does not increase the repo size, and anything that would be added here is already present in the repo. |
@Fishrock123 ... I was referring to all of the tools that have node_modules, eslint included. |
Wait, what else do we use it for here? Taking a second look, if that's all that's needed this isn't a big deal, I thought this brought in lots of modules...? |
@Fishrock123 This uses |
nice hack. If that works we should have it. LGTM |
Generally LGTM, running local appears to do exactly what it needs to do :-) |
@Fishrock123 @Trott Are you okay with the current node_modules/ situation in this PR, at least as a temporary solution? |
Add `added:` and `deprecated:` entries to buffer.md. These are incomplete (particularly for some of the ancient features), but correct to the best of my knowledge. This serves as a demonstration of how the `added:`/`deprecated:` metadata may be implemented in 'real' docs. PR-URL: nodejs#6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Add `added:` and `deprecated:` entries to buffer.md. These are incomplete (particularly for some of the ancient features), but correct to the best of my knowledge. This serves as a demonstration of how the `added:`/`deprecated:` metadata may be implemented in 'real' docs. PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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 <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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 <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Add checks that make sure the doctool parses metadata correctly. PR-URL: nodejs#6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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 <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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 <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
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: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Add `added:` and `deprecated:` entries to buffer.md. These are incomplete (particularly for some of the ancient features), but correct to the best of my knowledge. This serves as a demonstration of how the `added:`/`deprecated:` metadata may be implemented in 'real' docs. PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This commit introduces the Documentation YAML metadata concept to the documentation. - Parses added or Added for HTML - Parses all data for JSON Ref: #3867 Ref: #3713 Ref: #6470 PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Add checks that make sure the doctool parses metadata correctly. PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Add `added:` and `deprecated:` entries to buffer.md. These are incomplete (particularly for some of the ancient features), but correct to the best of my knowledge. This serves as a demonstration of how the `added:`/`deprecated:` metadata may be implemented in 'real' docs. PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This commit introduces the Documentation YAML metadata concept to the documentation. - Parses added or Added for HTML - Parses all data for JSON Ref: #3867 Ref: #3713 Ref: #6470 PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Add checks that make sure the doctool parses metadata correctly. PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Add `added:` and `deprecated:` entries to buffer.md. These are incomplete (particularly for some of the ancient features), but correct to the best of my knowledge. This serves as a demonstration of how the `added:`/`deprecated:` metadata may be implemented in 'real' docs. PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This commit introduces the Documentation YAML metadata concept to the documentation. - Parses added or Added for HTML - Parses all data for JSON Ref: #3867 Ref: #3713 Ref: #6470 PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Add checks that make sure the doctool parses metadata correctly. PR-URL: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #6495 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Checklist
Affected core subsystem(s)
tools, doc
Description of change
This is a continuation of #3867. Original description:
This commit introduces the Documentation YAML metadata concept to the
documentation.
This is in prelude to #3713.
Markup would look like this:
Changed/additionally included in this PR:
added:
attribute to be a collection of versions rather than a single one, since semver-minors can trickle down to previous major versions.deprecated:
attribute./cc @nodejs/documentation