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

doc: make stability labels more consistent #57516

Merged
merged 5 commits into from
Mar 23, 2025

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Mar 17, 2025

Our docs are not very consistent when it comes to Stability 1: Stable labels, for most occurrences each section inherits from its parent section, but in some cases there's a redundant label, and e.g. in globals.md or cli.md, many sections do not have any label at all, despite being de facto stable. This PR tries to fix all those inconsistencies.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/security-wg
  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 17, 2025
@marco-ippolito
Copy link
Member

So the rule of thumb is, if there is a parent it inherits the parent stability index, otherwise it needs to be declared

@aduh95
Copy link
Contributor Author

aduh95 commented Mar 17, 2025

So the rule of thumb is, if there is a parent it inherits the parent stability index, otherwise it needs to be declared

Yeah I think that's how we've treated it. I'll try to see if I can update the tooling to:

  • reject when redundant stability indexes are declared (to keep it consistent)
  • render in the HTML version the stability index to all the descendents (or if that makes it too busy, try to find a way to hint about the inherited status)

@aduh95
Copy link
Contributor Author

aduh95 commented Mar 19, 2025

The CSS part would need a bit more work, help would be welcome /cc @nodejs/nodejs-website

@avivkeller
Copy link
Member

The CSS part would need a bit more work, help would be welcome

Your CSS looks good to me, assuming I’ve understood it correctly. It ensures the stability header stays just below the navbar. However, if that’s not the intended behavior, I’m happy to take another look.

@aduh95
Copy link
Contributor Author

aduh95 commented Mar 21, 2025

You can see the unintended behavior e.g. in http://127.0.0.1:8000/url.html#class-urlsearchparams, where URLSearchParams appears as experimental, when it should inherit from the Stable status. I think it could be fixed by wrapping each section in the HTML

@avivkeller
Copy link
Member

avivkeller commented Mar 21, 2025

Well, the tooling for generating the documentation is about to change, (see #57343), and this CSS issue does not occur in that tooling (in fact, a different CSS issue occurs with the stability node not sticking for children elements) (see nodejs/api-docs-tooling#215). Honestly, I'd personally advice against changing the CSS at all, as in the coming months, the entire docs page is (as i'm sure you are aware), being redesigned. That being said, if you would still like to see this CSS change, I'd be happy to look into a solution for the issue you are facing in both this PR, and in the new tooling.

(By the way, I'm sure the website team can definitely look into ways of indicating inherited stability for the redesign)

@ovflowd
Copy link
Member

ovflowd commented Mar 21, 2025

Well, the tooling for generating the documentation is about to change, (see #57343), and this CSS issue does not occur in that tooling (in fact, a different CSS issue occurs with the stability node not sticking for children elements) (see nodejs/api-docs-tooling#215). Honestly, I'd personally advice against changing the CSS at all, as in the coming months, the entire docs page is (as i'm sure you are aware), being redesigned. That being said, if you would still like to see this CSS change, I'd be happy to look into a solution for the issue you are facing in both this PR, and in the new tooling.

(By the way, I'm sure the website team can definitely look into ways of indicating inherited stability for the redesign)

+1, @aduh95 I appreciate your commit and the time invested here, but the whole tooling is about to change. Could you defer any update till then? Changes are really really imminent 🙏

@aduh95 aduh95 marked this pull request as ready for review March 22, 2025 01:14
@aduh95
Copy link
Contributor Author

aduh95 commented Mar 22, 2025

I've added the wrappers, fixing #57516 (comment)

Changes are really really imminent 🙏

This PR is ready to land, I don't think it'd wise to wait for it to gather conflicts

@aduh95 aduh95 requested a review from marco-ippolito March 22, 2025 01:28
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@lpinca lpinca added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Mar 23, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 23, 2025
@nodejs-github-bot nodejs-github-bot merged commit 524c078 into nodejs:main Mar 23, 2025
25 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 524c078

aduh95 added a commit that referenced this pull request Mar 25, 2025
PR-URL: #57516
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
pandeykushagra51 pushed a commit to pandeykushagra51/node that referenced this pull request Mar 28, 2025
PR-URL: nodejs#57516
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 1, 2025
PR-URL: #57516
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 1, 2025
PR-URL: #57516
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants