-
Notifications
You must be signed in to change notification settings - Fork 15k
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
chore: use semantic-commit-action #33857
Conversation
.github/workflows/semantic.yml
Outdated
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
with: | ||
validateSingleCommit: true |
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.
Our current behaviour for this is false
. titleOnly
ignores the commit message for single commits and is the same as validateSingleCommit: false
Line 2 in fec147a
titleOnly: true |
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.
hm... okay. I'll switch it to false
to maintain existing behavior but I think we may actually want this to consider changing this to be true
because we have merged a bunch of non-semantic commits by accident. e.g. 92c5ded
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.
That's true. The trade-off is that folks are used to editing PR titles to make sure things are semantic and it annoyingly doesn't work with single-commit PRs.
Would be nice if squash and merge
always defaulted to PR title rather than commit message regardless of the number of commits in a PR, but that's for the product team to figure out.
As the (lousy) maintainer of the semantic pull requests app, I support this choice! Looks like it went down this time because Heroku's GitHub App tokens were compromised: https://status.heroku.com/incidents/2413 |
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 YML looks good - thanks for adding this!
No Release Notes |
I have automatically backported this PR to "15-x-y", please check out #33863 |
I have automatically backported this PR to "16-x-y", please check out #33864 |
I have automatically backported this PR to "17-x-y", please check out #33865 |
I have automatically backported this PR to "18-x-y", please check out #33866 |
I have automatically backported this PR to "19-x-y", please check out #33867 |
* chore: use semantic-commit-action * Update semantic.yml
Refs electron/electron#33857. Since this repo uses CFA releases, enforce semantic commit messages.
Refs electron/electron#33857. Since this repo uses CFA releases, enforce semantic commit messages.
Refs electron/electron#33857. Since this repo uses CFA releases, enforce semantic commit messages.
Description of Change
The semantic commit checker is
down, and not for
the first time. Let's switch to using GitHub Actions.
Notes: none