feat(cli): use special errorCode for missing rules/config #4142 #4143
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
@knocte wdyt? |
@commitlint/cli/src/cli.ts
Outdated
| @@ -384,6 +387,9 @@ async function main(args: MainArgs): Promise<void> { | |||
| throw new CliError(output, pkg.name, 2); | |||
| } | |||
| } | |||
| if (!report.valid && isRulesEmpty) { | |||
There was a problem hiding this comment.
@escapedcat if isRulesEmpty=true at line 347, do we really need to perform the invokation at line 348 or can return early?
There was a problem hiding this comment.
Tried that before. We need a "formatted" report (aka output) to display the current message.
We could just throw a new error but I kinda like the current formatted way.
There was a problem hiding this comment.
still, wouldn't it be better to check isRulesEmpty first? i.e. if (isRulesEmpty && (!report.valid)) {, micro-optimization there; or even just if (isRulesEmpty) {
There was a problem hiding this comment.
Got it, adjusted, thanks!
|
Cool, and let's refactor the exit codes to be an int-based enum? |
Done 👍 |
|
LGTM |
|
I'll wait for @ferrarimarco feedback and merge after. Is this breaking in any way? I'll go with a no here 🤞 |
|
Thanks for this PR. This is perhaps a better reference from the official Node docs: https://nodejs.org/api/process.html#exit-codes. Its content seem to match your link. From a end user point of view, this will work because we can check if the exit code is I don't know enough about commitlint internals for a deep review, but I'll try to leave some comments. Thanks a lot for working on this! |
Description
Fixes #4142
I wonder if this is a breaking change.
Used 9 after looking at this:
https://www.geeksforgeeks.org/node-js-exit-codes/
Is 9 correct?
How Has This Been Tested?
Currently it is notAdjusted existing tests
Types of changes
Checklist: