Skip to content
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

Make new single-commit PR validation optional #159

Closed
mowies opened this issue Feb 7, 2022 · 9 comments · Fixed by #160
Closed

Make new single-commit PR validation optional #159

mowies opened this issue Feb 7, 2022 · 9 comments · Fixed by #160

Comments

@mowies
Copy link

mowies commented Feb 7, 2022

The changes done by @kenji-miyake in #158 are a great addition, but I think they should be configurable through a flag/setting.

I have a few bot-created pull requests where the single commit message has more info that should stay there, but it's not important for the PR reviewer, so in this case I would like to keep the title simple but the commit message should still have more advanced info inside.

Here is an example: keptn/keptn#6789

@amannn
Copy link
Owner

amannn commented Feb 7, 2022

Ok, yes – sounds like a fair use case. @kenji-miyake Would you mind adding an opt-in flag like validateSingleCommitMatchesPr for the behaviour you've added?

@kenji-miyake
Copy link
Contributor

No, I don't mind that, but I feel we can discuss it a bit more here.

Although I don't deny @mowies's use case, let me confirm:

  • What if GitHub changes the specification of the squashed commit message?
    • In that case, the message will suddenly change from build(deps): Update keptn/go-utils to 8717cd9293a5940fd4dd931b9ea1daf16258364e to build(deps): Auto-update go-utils to latest version.
    • Is it acceptable for you?
  • What is the reason for not writing the commit message like this?
build(deps): Auto-update go-utils to latest version

Update keptn/go-utils to 8717cd9293a5940fd4dd931b9ea1daf16258364e

Anyway, I'll prepare a PR while waiting for the reply!

@kenji-miyake
Copy link
Contributor

@amannn Regarding the CI, should I create lint-pr-title-preview-validateSingleCommitMatchesPr.yml or can I add an option to lint-pr-title-preview-validateSingleCommit.yml?

@amannn
Copy link
Owner

amannn commented Feb 7, 2022

I think enabling the flag in the existing workflow should be fine. Thanks a lot!

@kenji-miyake
Copy link
Contributor

I'm considering what is the best description for this option... 🤔
Do you have any ideas?

@kenji-miyake
Copy link
Contributor

@amannn How about this one?

When using "validateSingleCommit", even if you edit the PR title, the PR title is not used for the merge commit.

Also, since it cannot detect an error as long as the commit message is semantic, it's easy to commit this by mistake.

Enable this option to also validate if the PR title matches the commit title.

kenji-miyake pushed a commit to kenji-miyake/action-semantic-pull-request that referenced this issue Feb 7, 2022
@github-actions
Copy link

github-actions bot commented Feb 8, 2022

🎉 This issue has been resolved in version 4.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@kenji-miyake
Copy link
Contributor

FYI: A new option, "Default to PR titles for squash merge commit messages".
https://github.blog/changelog/2022-05-11-default-to-pr-titles-for-squash-merge-commit-messages/
community/community#16271

@amannn
Copy link
Owner

amannn commented May 12, 2022

Oh my god, that's fantastic. Thank you so much for sharing this @kenji-miyake! I'll add this to the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants