-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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
cli: add NODE_RUN_SCRIPT_NAME
env to node --run
#53032
cli: add NODE_RUN_SCRIPT_NAME
env to node --run
#53032
Conversation
e5953cf
to
69dbfd7
Compare
d1dbd64
to
f26e6dd
Compare
f26e6dd
to
073643e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
073643e
to
612181d
Compare
I don't understand the name. What's the connection between running a script and the concept of a "lifecycle event"? |
I don't understand it either. It's named after "npm_lifecycle_event" environment variable that is required for CLI runners to detect which task is being run. The reference issue in PR description might give you more context. |
Lifecycle events make sense in the context of a package manager, but since this feature apparently doesn't aim for compatibility with any existing package manager or runtime, I think it'd be acceptable to deviate from existing naming conventions. |
That seems valid to me. @tniessen. Any naming suggestions? |
An environ named as something like |
node --run
NODE_RUN_COMMAND_NAME
env to node --run
612181d
to
5afff43
Compare
Sorry about the churn. Do we have a consensus on "command" vs "script"? I found both in the doc and codebase, but |
I'm not necessarily in favor of any of this, but |
NODE_RUN_COMMAND_NAME
env to node --run
NODE_RUN_SCRIPT_NAME
env to node --run
5afff43
to
1ae34ba
Compare
(the ask and solution seem reasonable to me) |
Landed in cb90a31 |
PR-URL: nodejs#53032 Refs: nodejs#52673 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Adds the
NODE_RUN_SCRIPT_NAME
to share name of the event that's run while executingnode --run
. For example, if the developer runsnode --run yagiz
,NODE_RUN_SCRIPT_NAME
will beyagiz
This PR also includes a small refactor. I moved setting environment variables into
SetEnvironmentVariables
private function to improve readability.Ref: #52673
cc @nodejs/cpp-reviewers