chore(refactor): clean up lifecycle-cmds#2753
Closed
Conversation
darcyclarke
added
the
Release 7.x
There are a few commands (exec, run-script, and the run-script proxies) where essentially npm is acting like a very fancy shell. It is peculiar and noisy for npm to print a verbose error banner at the end of these commands, since presumably the command itself already did whatever it had to do to report the error appropriately. For example, `npm test` runs a test script, usually outputting test results. Having npm then tell me that my tests failed with exit status 1 and print a debug log, is unnecessary and unwanted. When the error encountered for these commands does not have a non-zero numeric 'code', then we still print the standard npm error reporting messages, because presumably something went wrong OTHER than a process exiting with a non-zero status code. PR-URL: #2742 Credit: @isaacs Close: #2742 Reviewed-by: @nlf
This is a small incremental step in removing some of the complexity surrounding the `npm` object in our code. It moves that object one level up the chain of code, with the end goal being that we will only require it once in the codebase, ultimately allowing us to improve our tests. I also changed the command that it calls to `run-script` because that is the base command. `run` was an alias, and that is one more layer of hoops a developer would have to jump through to find what file in `./lib` is even being referenced here. PR-URL: #2753 Credit: @wraithgar Close: #2753 Reviewed-by: @isaacs
isaacs
force-pushed
the
gar/lifecycle-cmd-refactor
branch
from
481fbae to
773ae3e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a small incremental step in removing some of the complexity
surrounding the
npmobject in our code.It moves that object one level up the chain of code, with the end goal
being that we will only require it once in the codebase, ultimately allowing
us to improve our tests.
I also changed the command that it calls to
run-scriptbecause that is thebase command.
runwas an alias, and that is one more layer of hoops adeveloper would have to jump through to find what file in
./libis evenbeing referenced here.