-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
feat(ci): request reviews from API experts #8040
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8040 +/- ##
==========================================
+ Coverage 76.22% 76.29% +0.06%
==========================================
Files 118 118
Lines 9903 9903
Branches 337 336 -1
==========================================
+ Hits 7549 7555 +6
+ Misses 2352 2346 -6
Partials 2 2 ☔ View full report in Codecov by Sentry. |
38b5ff3 to
d43388a
Compare
|
Requesting a review from the following teams:
|
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 implements a new automated reviewer assignment system for Node.js documentation by mapping articles to API domains and their respective expert teams. The change replaces static CODEOWNERS assignments with dynamic review requests based on frontmatter metadata.
- Adds
apifrontmatter field to 51 learning documentation files to categorize content by Node.js API domain - Creates automated GitHub workflow to request reviews from appropriate API expert teams
- Removes static CODEOWNERS entries for TypeScript and security content in favor of the new dynamic system
Reviewed Changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Multiple learn/*.md files | Added api frontmatter field to categorize content by Node.js API domain |
| .github/workflows/request-review.yml | New workflow to automatically request reviews based on API categorization |
| .github/scripts/get-reviewers.mjs | Script to parse frontmatter and map APIs to reviewer teams |
| .github/reviewers.json | Configuration mapping API domains to GitHub teams |
| .github/CODEOWNERS | Removed static assignments for TypeScript and security content |
Comments suppressed due to low confidence (1)
.github/scripts/get-reviewers.mjs:40
- [nitpick] The parameter name 'content' is too generic. Consider renaming it to 'fileContent' or 'markdownContent' to better indicate what it contains.
if (!content.trimStart().startsWith('---')) {
ad6bbae to
f1b6706
Compare
apps/site/pages/en/learn/asynchronous-work/event-loop-timers-and-nexttick.md
Outdated
Show resolved
Hide resolved
|
Lighthouse Results
|
Bump @nodejs/web-admins |
|
@ovflowd @bmuenzenmeyer bump x2 |
|
Sorry. Will try to look soon. Tuesdays and Wednesdays are tough for me |
bmuenzenmeyer
left a comment
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
713fac0 to
91f61f3
Compare
|
Waiting for:
To complete testing |
@bmuenzenmeyer can you enable this action? |
@avivkeller done |
This comment was marked as resolved.
This comment was marked as resolved.
24090ee to
ad99133
Compare
This comment was marked as resolved.
This comment was marked as resolved.
70601c2 to
4cf9907
Compare
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/web-infra Please review the changes when you have a chance. Thank you! 🙏 |
4cf9907 to
cd9d959
Compare
|
Testing looks good! Note that the comment was made multiple times in this PR for two different tests, in a real scenario, the comment will only be made on PR open |
Description
This is a potential solution for the linked issues, via mapping article -> api -> owner.
Prereqs:
@nodejs/typescriptwrite access revoked@nodejs/security-wgwrite access revoked1Validation
See #8040 (comment). In a real PR, these teams will be pinged.
Related Issues
Fixes #7292
Fixes #7294
Footnotes
Unless they have it for a different reason ↩