-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
esm: fix globalPreload
warning
#49069
Conversation
Review requested:
|
No, we need to keep the language about it being removed in a future version. |
Why? |
Because it's stronger. It means you must refactor, it's not just a recommendation. They should know that this is coming. |
I agree that the existing verbiage is better than the proposal because we're pretty darn sure upgrading is required (whereas the proposal makes it sound like merely a better option). |
Well we plan to remove it, but we can't know for sure if we are definitely going to remove it.
Surely you don't mean that, the current warning is not correct English. |
I assume he means that the gist of the current warning is better than the proposed alternative. As in, just fix the grammar error without changing the intent of what we're trying to say. |
Yes, the minor typographic error aside, the current is better information for the user. I think it would be better to correct the current message. |
What would be your suggestion? |
Landed in b5da2f4 |
PR-URL: nodejs#49069 Fixes: nodejs#49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: #49069 Fixes: #49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: nodejs#49069 Fixes: nodejs#49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: nodejs#49069 Fixes: nodejs#49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: #49069 Fixes: #49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: #49069 Fixes: #49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: nodejs#49069 Fixes: nodejs#49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: #49069 Backport-PR-URL: #50669 Fixes: #49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: nodejs/node#49069 Backport-PR-URL: nodejs/node#50669 Fixes: nodejs/node#49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
PR-URL: nodejs/node#49069 Backport-PR-URL: nodejs/node#50669 Fixes: nodejs/node#49026 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
Fixes: #49026
Before:
After: