-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
meta: deduplicate dependencies #8499
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.
|
👋 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! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8499 +/- ##
=======================================
Coverage 74.26% 74.26%
=======================================
Files 106 106
Lines 9102 9102
Branches 307 307
=======================================
Hits 6760 6760
Misses 2340 2340
Partials 2 2 ☔ View full report in Codecov by Sentry. |
📦 Build Size ComparisonSummary
Changes➕ Added Assets (4)
➖ Removed Assets (4)
|
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)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
👀 @avivkeller, ideally, I'd prefer this to be merged before any of the open Dependabot PRs, just so that I don't need to essentially recreate this and have folks review the diff again? |
ovflowd
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.
SGTM
|
(My SGTM is just a sign-off, I haven't tested the PR yet) |
|
LGTM, but can we wait until after the Dependabot PRs land, since it's possible they will re-introduce some duplicated dependencies? |
|
Oh true, sure, I can recreate this once they're landed then. Don't worry about reviewing this until after this is recreated then, as I imagine the diff will change. |
|
@MattIPv4 can you rebase this? I did it locally too but unsure how you feel about me force pushing onto your fork/branch |
|
Are we planning to merge #8485 first, or is that going to stay open due to the failing OpenNext CI? |
|
ope - missed that one in my scan - I think we should always be careful with next and open next compatibility, so I would be fine omitting it |
85d206d to
4f07be2
Compare
Description
Generated by
pnpm dedupeat the root.Changed dependencies: https://github.com/nodejs/nodejs.org/actions/runs/20666412561/job/59339511591?pr=8499#step:3:15
Validation
All the lockfile changes here should be very carefully reviewed, and the functionality of the site should be thoroughly tested, given the number of dependencies that've changed (though of course all within semver compliance).
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.