Skip to content

chore(deps): re-sync lockfile #7634

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

Closed
wants to merge 3 commits into from
Closed

chore(deps): re-sync lockfile #7634

wants to merge 3 commits into from

Conversation

avivkeller
Copy link
Member

@avivkeller avivkeller commented Apr 9, 2025

Description

This is a chore re-sync of the lockfile. It's been a while since this file was re-generated, and it's important (IMO) to do it every once in a while.

All tests pass, storybook looks fine, and nothing crashes AFAICT on Chrome + Safari.

Validation

Nothing should change, and no tests should fail

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npm run format to ensure the code follows the style guide.
  • I have run npm run test to check if all tests are passing.
  • I have run npx turbo build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

@Copilot Copilot AI review requested due to automatic review settings April 9, 2025 17:37
@avivkeller avivkeller requested a review from a team as a code owner April 9, 2025 17:37
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Copy link

vercel bot commented Apr 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Apr 9, 2025 5:48pm

Copy link
Contributor

github-actions bot commented Apr 9, 2025

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🟢 100 🟢 100 🟢 100 🟢 91 🔗
/en/about 🟢 100 🟢 100 🟢 100 🟢 91 🔗
/en/about/previous-releases 🟢 100 🟢 100 🟢 100 🟢 92 🔗
/en/download 🟠 89 🟢 100 🟢 100 🟢 91 🔗
/en/blog 🟢 100 🟢 100 🟢 96 🟢 92 🔗

Copy link
Contributor

github-actions bot commented Apr 9, 2025

Unit Test Coverage Report

Title Lines Statements Branches Functions
@node-core/ui-components Coverage: 95%
95.83% (161/168) 77.86% (102/131) 88.57% (31/35)
@nodejs/website Coverage: 87%
84.7% (493/582) 76.03% (165/217) 86.88% (106/122)
Title Tests Skipped Failures Errors Time
@node-core/ui-components 24 0 💤 0 ❌ 0 🔥 4.736s ⏱️
@nodejs/website 156 0 💤 0 ❌ 0 🔥 6.332s ⏱️

@draclyr
Copy link

draclyr commented Apr 9, 2025

good work❤️

Copy link
Collaborator

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm -1 on the practice, as it's imprecise and very hard to review.

@AugustinMauroy
Copy link
Member

I didn't get what is the issue why "updating" the lock file ?

@avivkeller
Copy link
Member Author

Regenerating the lockfile ensures that the package versions we install are the best ones available. Over time, the file can slightly differ from its optimal state, mainly because Dependabot updates only a few packages at a time in isolation, not taking into account if a specific package has a better version available that suits our specified requirements.

Copy link
Collaborator

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

putting an explicit block on this til we have consensus

@MattIPv4
Copy link
Member

I am -1 to this practice -- I have never encountered it before as a recommended thing to do, and it seems incredibly risky / complex to review. If we're going to do this, why have a lockfile at all? I'd much rather see PRs to bump individual transitive dependencies that we know need updating for a specific reason, and beyond that just let them naturally update as direct dependencies require.

@avivkeller
Copy link
Member Author

avivkeller commented Apr 11, 2025

I think enough of us have said -1 to this PR that it shouldn't land, and we should continue discussion on https://openjs-foundation.slack.com/archives/CVAMEJ4UV/p1744401326173889.

Sorry for the noise, everyone!

@avivkeller avivkeller closed this Apr 11, 2025
@avivkeller avivkeller deleted the chore/resync-lockfile branch April 11, 2025 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants