Exit without error message when requireCommits is true but requireCommitsFail is false and no commits are available #1099
Conversation
…ireCommitsFail is false and no commits are available Fixes release-it#1095
|
Looking at tests right now :) |
|
Hey @webpro, the change is simple, but the tests are failing what I can see is for not being able to assert correctly. I am checking but I cannot see to be able to make it work as expected. Do you have any tips on where to change it I was looking on the factory https://github.com/release-it/release-it/blob/main/test/util/index.js#L11 but never used ava before so learning as I go :) |
|
Ok, I sorted out the other ones, but one due to ava test not being a big fan of process.exit. Checking what to do... |
|
I have a different solution that goes into the message instead of the check. I prefer the solution on this MR, but lets see which one is more ideal juancarlosjr97@a381972 but as this solution still uses the throw all the tests are still passing https://github.com/juancarlosjr97/release-it/actions/runs/8765416893. Let me know your thoughts @webpro :) |
| } | ||
|
|
||
| this.log.info(`${requireCommitsFailMessage}${EOL}Documentation: ${docs}${EOL}`); | ||
| exit(0); |
There was a problem hiding this comment.
Not sure if exiting the process here is the best way to go about this, as it's a bit one-off/hidden.
What about the using the third fail boolean argument, attach it to the error object, and then use that to decide whether it should use log.error or log.info in the catch handler of the main sequence in lib/index.js?
There was a problem hiding this comment.
Some like I did here as I am using a third argument to change the message type juancarlosjr97@a381972?
There was a problem hiding this comment.
Yes, although I'm not super fond of using "name" to distinguish the error log type. Did you see that used somewhere else?
There was a problem hiding this comment.
No, it was just an idea but I do not like it either because it mutation the error object to show different messages. I did not have another idea without going into refactoring which is probably the best course to take.
Using the exit(0); somehow I think is nicer as the program stops regardless but with a different handled.
I had an idea but I did not even put it into action throwing info instead that was wrong in all places.
There was a problem hiding this comment.
You could use error.details or error.cause to attach things like this, I think that's fairly common.
There was a problem hiding this comment.
Yes, that sounds better!
|
Closed this MR for #1101 :) |
Change
Updating the flow to when
requireCommitsis true butrequireCommitsFailis set as false and no commits are available, instead of returningERROR There are no commits since the latest tag.just returnsThere are no commits since the latest tag.and remove theERRORstring to as that is misleading.Fixes #1095