-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
build: remove the rest of corepack #57763
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
base: main
Are you sure you want to change the base?
Conversation
Review requested:
|
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, adding my block to make sure this doesn't land before v24.x is in Maintenance
@aduh95 that's fine, but can you help me understand why main would affect v24, which this commit wouldn't land on at all? |
Commits make their way to LTS lines by being first released onto a Current release for at least two weeks, and Current releases get their commits from |
Unfortunately the previous discussion, and this one, didn't make it clear to me why this is a burden. I'm aware that commits will land on main before being backported to v24, but the things this PR touches are highly unlikely to conflict with anything destined for v24 or older, so I'm unclear on what maintenance burden there would be. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57763 +/- ##
=======================================
Coverage 90.24% 90.24%
=======================================
Files 630 630
Lines 185204 185204
Branches 36296 36292 -4
=======================================
+ Hits 167133 167144 +11
+ Misses 11003 10997 -6
+ Partials 7068 7063 -5 🚀 New features to boost your workflow:
|
There's no point in releasing in a Current line that doesn't have the feature. I don't believe it will be more difficult to maintain if the code only lives on <=v24.x. We'll just need to change automation to open the update PR against v24.x-staging instead of main. |
Of course it would be, PR targeting
For end users, sure. For us maintainers, the point is to keep using the existing process and minimize manual interventions. |
I think I understand the issue - because of the policy that anything destined for Current or LTS has to sit on main for 2 weeks, commits updating corepack have to land on main, which means this PR can't land on main. Either blocking this PR until 24 is in maintenance would work, or, altering the policy so that some kinds of commits don't have to hit main before landing in Current/LTS. |
To clarify, the policy is not blocking, we could still manage pushing updates to specific branches, the Release WG would likely won't see any problem with that. However, as explained in my previous comment, it would make Corepack maintenance harder / give more work to the releasers, so I'm against it. |
Refs:
This fulfills the spirit of the TSC vote by removing the ability to build node with corepack at all, including removing the vendored dep. It should only land in node 25+, and should not be backported (which should also mean that corepack distribution in release lines < v25 are not disrupted).
It leaves behind
corepack.md
.