-
Notifications
You must be signed in to change notification settings - Fork 240
chore: add conditional check to preview release #5457
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
Conversation
|
| runs-on: ubuntu-latest | ||
|
|
||
| # Run the job if manually triggered or if the commit message includes a commit type of fix or feat or breaking change | ||
| if: contains(github.event.head_commit.message, 'fix:') || contains(github.event.head_commit.message, 'fix(') || contains(github.event.head_commit.message, 'feat:') || contains(github.event.head_commit.message, 'feat(') || contains(github.event.head_commit.message, '!:') |
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.
using the : and ( ensures it is a commit type and not just a random word in the message
Branch previewReview the following VRT differencesWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
If the changes are expected, update the |
nikkimk
left a comment
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.
Lawless genius twirls mustache (LGTM)
Tachometer resultsCurrently, no packages are changed by this PR... |
graynorton
left a comment
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.
This looks like it will work, and I'm not necessarily opposed to going with this approach, but it bothers me a little bit to be relying on conventional commits for the publish-or-not logic, since we have moved in the direction of having Changesets be our source of truth for representing release-worthy changes.
What if, instead of conditionalizing the logic inside the job and inspecting commits, we were to only run the job when a push to main includes changes beneath the .changesets directory, something like this:
on:
push:
branches:
- main
paths:
- '.changesets/**'
I think we could also add workflow_dispatch: to the on: block if we wanted to allow manually triggering the action from the GitHub UI.
we provide the |
Ah, I hadn't thought about the missing sha issue! The use case I had in mind was not an one-off snapshot release (which I agree Anyway, it may be that we don't need to worry about that case. |
graynorton
left a comment
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.
Approved, so we can get this merged. We'll follow up later to explore the possibility of using a Changesets-based trigger, as discussed here.
* chore: add conditional check to preview release * chore: add breaking change conditional
Description
This adds a conditional check to the
preview-releaseworkflow to only run on commit typesfeat,fix, or!:. The!:will capture any breaking change.Motivation and context
Related issue(s)
Author's checklist
Reviewer's checklist