outdated: exitcode != 0 on outdated dependencies#14013
outdated: exitcode != 0 on outdated dependencies#14013watilde wants to merge 1 commit intonpm:masterfrom
Conversation
This update is a breaking change feature. When outdated dependencies exist in a project, the command displays a helpful error message and then finish the action with `exit 1`. To ignore the error and keep the previous behavior, it's possible via passing `--silent` option. Fixes: npm#5999 PR-URL: npm#14013 Credit: @watilde Reviewed-By: @
2c017eb to
41e36e8
Compare
iarna
left a comment
There was a problem hiding this comment.
For the record: this all looks good. We'll be including it in npm@4.
| 'the versions in node_module, or look over package.json', | ||
| 'to use the most suitable semver range.' | ||
| ].join('\n') | ||
| cb(msg, list.map(function (item) { return [item[0].parent.path].concat(item.slice(1, 7)) })) |
There was a problem hiding this comment.
Quick question – should the first arg here be an Error instead of a String? (not sure about npm's style preference – but most Node.js projects typically use Errors so that stack information is retained, among other things)
There was a problem hiding this comment.
Errors from commands (like this one) are processed via lib/utils/error-handler.js, which, if the error argument is a string, prints it to the user using log.error and exits with a non-zero exit code.
By contrast, if we called back with an error object (which we often do), then its output message is determined via lib/utils/error-message.js and it's logged along with a bunch of other diagnostic information.
This is actually the only way currently to trigger a non-zero exit code without printing all sorts of diagnostic information on folks screens.
If I were to design our error handling, I'd have it ONLY accept error objects and use an error class to use for this kind of scenario. But IMO that kind of rewrite (while positive) is out of scope for this PR and so I wouldn't ask someone contributing a change like this one to make that kind of rewrite.
This update is a breaking change feature. When outdated dependencies exist in a project, the command displays a helpful error message and then finish the action with `exit 1`. To ignore the error and keep the previous behavior, it's possible via passing `--silent` option. Fixes: #5999 PR-URL: #14013 Credit: @watilde Reviewed-By: @zkat Reviewed-By: @iarna
|
This was merged into We had some discussion at the last minute about the UX for this and I ended up making some quick patches on top of what you did in a separate PR, btw. If we'd had more time pre-npm@4 we would've done it in review though. ^_^;; This is really great, though, @watilde! Thank you as always for the excellent work <3 |
|
FYI, I don't see tests for Am I missing something? |
Fixes: #5999.
This update is a breaking change requested a long time ago. When outdated dependencies exist in a project, the command displays a helpful error message and then finish the action with
exit 1. To ignore the error and keep the previous behavior, it's possible via passing--silentoption.