-
-
Notifications
You must be signed in to change notification settings - 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-kit
is finished, we can follow up in node core to replaceremark-preset-lint-node
with@nodejs/remark-lint/api
.Our current setup is fragmented:
remark-preset-lint-node
doc-kit
remark-preset-lint-node
between 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
version
package.json field is bumped (or viaworkflow_dispatch
).cc @nodejs/web-infra
cc @nodejs/linting
cc @Trott
Blocked:NPM_TOKEN
secret needs to be updated to allow publishing@node-core/remark-lint