-
Notifications
You must be signed in to change notification settings - Fork 35
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
"any" value added to target option #99
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we make any the new default?
yeah I think |
@simoneb is this PR good to merge? I had updated the test cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They way you updated the tests doesn't look right, the structure of the nested tests specifically. Please have a look
The nested tests were already there. I have eliminated the nesting and improved the structure now. Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nesting made sense, it just didn't make much sense after you had changed it.
It made sense because previously we returned major when:
- major was provided
- input was not recognized
With your change "any" would have got the same behavior as major had previously but you didn't update the structure of the tests accordingly.
Without nesting the problem does not exist in the first place so we're good
The default value for `target` was still `major` but it has been changed to `any` here fastify#99.
The default value for `target` was still `major` but it has been changed to `any` here #99.
The target value can be one of the
major, premajor, minor, preminor, patch, prepatch or prerelease
. This PR adds another valueany
which can be used to run the action without any semver version checks.Documentation has been updated
Closes #95