-
Notifications
You must be signed in to change notification settings - Fork 134
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
Start a vote on require(esm) markers #1621
Conversation
This is a follow-up to the naming bikeshed in nodejs/loaders#221 which is for the implementation in nodejs/node#54563 A summary can be found in nodejs/node#54563 (review)
cc @nodejs/tsc @nodejs/loaders @guybedford |
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.
Thanks @joyeecheung for putting this together. Just added some extra notes on the add-on PR here, as it is also part of the implications here.
Co-authored-by: Guy Bedford <guybedford@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
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.
Thanks @joyeecheung for taking on board the feedback, happy to go ahead, and as I say I completely commit to following whatever is voted on here.
Co-authored-by: Guy Bedford <guybedford@gmail.com>
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
I am not entirely sure how long this should wait, https://github.com/nodejs/TSC/tree/main/votes/initiateNewVote#initiate-a-new-tsc-vote just says "Once the PR has approvals", and it's been open for about 2 days with a few approvals, so I guess I'll just merge it when it passes the 48-hour mark. |
This is a follow-up to the naming bikeshed in nodejs/loaders#221 which is for the implementation in nodejs/node#54563 A summary can be found in nodejs/node#54563 (review)