-
Notifications
You must be signed in to change notification settings - Fork 66
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
ci: improve release workflows to skip early #198
Conversation
# "commits" contains array of objects where one of the properties is commit "message" | ||
# Release workflow will be skipped if release conventional commits are not used | ||
if: | | ||
startsWith( github.repository, 'asyncapi/' ) && |
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.
to make sure workflow does not run on forks, to not confuse people
I already had few questions from folks asking why release is failing (but it was running on their forks)
# Release workflow will be skipped if release conventional commits are not used | ||
if: | | ||
startsWith( github.repository, 'asyncapi/' ) && | ||
(startsWith( github.event.commits[0].message , 'fix:' ) || |
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.
unfortunately github.event.commits[0].message
must be repeated several times. When I added it as global env in workflow, it was not accessed in condition
(startsWith( github.event.commits[0].message , 'fix:' ) || | ||
startsWith( github.event.commits[0].message, 'fix!:' ) || | ||
startsWith( github.event.commits[0].message, 'feat:' ) || | ||
startsWith( github.event.commits[0].message, 'feat!:' )) |
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.
unfortunately startsWith
has to be done this way, it accepts only string
as input for evaluation. In case of contains
it accepts array.
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.
@fmvilas sorry, please approve again. When I was in the sauna yesterday, I was thinking about this workflow and something told me to do some tests on the personal repo. And yeah, as a result, I had to rework the entire condition
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.
Yeah, that makes sense. I mean, the PR, not the sauna thing 😄
/rtm |
Description
semantic-release
finds out release is not neededstartsWith(fromJson
and notcontains(fromJson
like in other workflows because commit message can be multiline, and we do not have 100% assurance that PR was not manually merged and feat/fix was not mentioned somewhere in the messagestartsWith
also works withcontains
as there is not direct info in docs, at least I did not find it. I just searched github and found https://github.com/insignias/React/blob/main/.github/workflows/merge-release-pr-to-main.yml#L40Related issue(s)
Resolves #184