-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
doc: list the stability status of each module #36223
Conversation
I'm not sure something like this is that useful for at least a couple of reasons:
|
@mscdex Thank you for your feedback.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, though @mscdex 's idea of adding icons to the sidebar to indicate stability also sounds good. Any thoughts? @nodejs/documentation
some improvements possible (splitting the exact API that is non-stable into separate rows etc) , but thinking more about , it may be difficult to maintain - this is simple and useful as is. |
It looks like Modules is not fully covered. There are a variety of APIs that have stability levels including
I think this goes back to @cjihrig's comment that this isn't entirely accurate for all cases as some modules have methods that are deprecated or experimental. There is also an inconsistency here between the section title and the API name. To be clear, this is great work and I'm mostly pointing out edgecases. I won't block, but it would be nice to see some of these cases worked through |
e1937d8
to
bd75e2c
Compare
@MylesBorins Thank you for your feedback. I updated the code so that the section title and API name are now identical and in lowercase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While there will always be ways to improve it and I don't want to discourage those improvements, this is an improvement as it stands and so I'd be fine with landing it, especially since the table is maintained programmatically and so should not be a maintenance annoyance that gets out of synch with other docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great iterative improvement. Definitely places to improve, but that will drive more contributions to node!
LGTM
|
Co-authored-by: Antoine du Hamel <[email protected]>
Co-authored-by: Antoine du Hamel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, my suggestion was flawed – we need to specify the position, otherwise it sometimes jump to the end of the file.
Co-authored-by: Antoine du Hamel <[email protected]>
Co-authored-by: Antoine du Hamel <[email protected]>
Co-authored-by: Antoine du Hamel <[email protected]>
Landed in 2bb42bf |
Fixes: #23723 PR-URL: #36223 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Fixes: #23723 PR-URL: #36223 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Fixes: #23723 PR-URL: #36223 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Fixes: #23723 PR-URL: #36223 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Fixes: #23723
Generate a module compatibility table from out/doc/all.json and insert it into documentation.html/md/json.
Preview:
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes