-
-
Couldn't load subscription status.
- Fork 6.4k
feat(remark-lint): add #8057
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(remark-lint): add #8057
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
The only lint changes are escaping characters (i.e |
4ca85a5 to
a8a258b
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8057 +/- ##
==========================================
+ Coverage 73.17% 75.84% +2.66%
==========================================
Files 97 112 +15
Lines 8440 9433 +993
Branches 228 304 +76
==========================================
+ Hits 6176 7154 +978
- Misses 2263 2278 +15
Partials 1 1 ☔ View full report in Codecov by Sentry. |
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.
Where's the llm_description lint rules?
🤦I'll add them today, whoops |
|
Is the idea that this is what will also be used to lint non-documentation .md files in the nodejs/node and nodejs/TSC repositories? Or will those continue to use remark-lint-preset-node and this is for website linting only? |
The idea is for this to be the linter associated with So, the web-infra and website teams will maintain a linter that complies with the generators we wrote (to ensure that they integrate well together). Yes, it'll work on the other repositories, but there's no obligation to use it there. |
|
It's going to have two presets.
|
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.
Am I missing a place or decision for us to have/not have READMEs in our packages? Seems like some of the context of this PR and the discussion already would be great to document as intents.
I noticed that the other packages don't have READMEs either. I don't intent to scope creep you but this seems like a good idea, no?
More review coming.
Issue on /admin repo? |
Yes, I'll open one, once this gets some approvals |
a8e566c to
ce80c49
Compare
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.
Mostly LGTM, but I feel our YAML rules could receive a bit of cleanup.
f854b17 to
91a19c0
Compare
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.
Pull Request Overview
This PR adds a new @node-core/remark-lint package that consolidates Node.js documentation linting rules into a centralized location. The package includes both base linting rules and API-specific rules that were previously scattered across different repositories.
Key changes:
- New remark-lint package with comprehensive validation rules for YAML comments, metadata, and Node.js documentation standards
- Migration of the site configuration to use the new centralized package
- Enhanced CI workflow to handle versioned packages properly
Reviewed Changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/remark-lint/* | Complete implementation of the new remark-lint package with rules, tests, and configuration |
| apps/site/.remarkrc.json | Simplified configuration to use the new centralized package |
| apps/site/package.json | Updated dependencies to use the new package |
| .github/workflows/publish-packages.yml | Enhanced CI to handle versioned packages correctly |
| apps/site/pages//**.md | Minor formatting fixes in documentation files |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
All in all lgtm! I have some non blocking questions but otherwise the tests look great and I'm excited for this to land.
8d64751 to
0c89552
Compare
|
Lighthouse Results
|
Fixes nodejs/doc-kit#350
This PR updates the monorepo to include a rewrite of
remark-preset-lint-node. Now that we've migrated to the new generators, it makes sense to centralize our linting rules in one place, maintained by the same team. This ensures the generator syntax aligns exactly with the linting configuration.The updated preset includes all existing rules plus the custom ones currently defined in
doc-kit. With a follow-up indoc-kit, this will also address nodejs/doc-kit#328.Once the migration to
doc-kitis finished, we can follow up in node core to replaceremark-preset-lint-nodewith@nodejs/remark-lint/api.Our current setup is fragmented:
remark-preset-lint-nodedoc-kitremark-preset-lint-nodebetween general and API-specific rulesBy consolidating everything into a single package, we’ll simplify future maintenance.
Tip
This package is versioned, and will only publish when the
versionpackage.json field is bumped (or viaworkflow_dispatch).cc @nodejs/web-infra
cc @nodejs/linting
cc @Trott
Blocked:NPM_TOKENsecret needs to be updated to allow publishing@node-core/remark-lint