-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
Revert of nodejs/node#55623 #56145
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Comments
@nodejs/tsc Any opinion how to progress in this? |
I don't think this change is affected by https://github.com/nodejs/node/blob/main/doc/contributing/collaborator-guide.md?rgh-link-date=2024-11-30T19%3A01%3A50Z#unintended-breaking-changes. |
Well, the result of e.g. In general I'm fine with going ahead and merge the fix and let it go it's path into 23. I assume that implies also that the |
Should we transfer this to nodejs/node? I don't think it's up to the TSC as long as there's consensus among collaborators. |
It's more about semver major yes/no. |
It looks like a bug, I also agree that this should be in nodejs/node. You can send a PR to revert and maybe find a releaser who's willing to volunteer to do a patch release, unless someone stops the revert and a consensus cannot be reached, the ball isn't really in TSC's court. |
ok, in that case I will go ahead, approve the fix PR and let it flow into release. I see no need for a revert. |
The PR #55623 caused a new issue: #56002.
#55623 landed already in 23.2.0 and to my understanding of contributor docs here a revert of a change already part of the releaes requires a TSC decission/discussion.
There is some history on above PR:
npm pack
crash in Node 23 #55410 and was reverted via Revert "path: fix bugs and inconsistencies" #55414I don't have the needed windows know how to actually fix the problem on short notice.
Which direction should we go? Revert or wait a bit for a fixup?
The text was updated successfully, but these errors were encountered: