-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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(about): list official resources #7562
Conversation
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.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (1)
- apps/site/pages/en/about/index.mdx: Language not supported
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
See the edited About page: https://nodejs-org-git-feat-official-resources-openjs.vercel.app/en/about |
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
cc @nodejs/tsc as this is content updates. I feel confident with these updates, but just pining y'all in. |
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 !
I've removed mention to other packages for now. If it's agreed upon that they should be listed, and someone can provide a definitive list, I'll add them into this PR. Also see nodejs/admin#959 |
Hey TSC! Have you reached a consensus on whether or not we need to include a list of packages in this PR? |
@nodejs/tsc |
I'm excelating this to the TSC agenda, as they are meeting in a few days (nodejs/TSC#1712). If they reach consensus before said date, or there is a change of heart on the addition of the agenda label, feel free to remove |
We discussed briefly during the TSC meeting and it was not clear what you are expecting from the group. Given that multiple TSC members have been involved in the PR and there are no blocks, we considered this a ping for awareness. |
Apologies if that was unclear — we were seeking a consensus on whether to list individual packages or only organization-controlled accounts. Seeing as there are no blocks, we will continue with only listing accounts for now. |
d452ad9
to
e174656
Compare
Please re-review with any final concerns, and merge-at-will. |
Lighthouse Results
|
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
Description
This PR lists official resources on the “About” page, so that users are steered away from unofficial, and possibly dangerous, resources.
I’m not sure if the TSC needs to sign off on this, to make sure that everything that should be included is.
Validation
Check the “About” page for the added information
Related Issues
Fixes #7050
Check List
npm run format
to ensure the code follows the style guide.npm run test
to check if all tests are passing.npx turbo build
to check if the website builds without errors.