zeke / semantic-pull-requests

:robot: Let the robots take care of the semantic versioning
https://github.com/apps/semantic-pull-requests
Apache License 2.0
1.24k stars 122 forks source link

Single commit PRs blocked #106

Closed Nadock closed 2 years ago

Nadock commented 4 years ago

Hi there,

The most recent release changed the handling of single-commit PRs that have a semantic title but not the commit. It now blocks one commit PRs that don't have a semantic commit title. (Issue #17, PR #105)

The only apparent work around was to add a second (whitespace) commit to the PR.

Is there anyway we can disable this behaviour? Previously, we'd just been copying the title into the squash-and-merge title, and that has been working well for us. We'd prefer not to need small whitespace commits in every single-commit PR.

From my reading of the #105 it looked like including a settings file would disable this behaviour, but that didn't change anything.


FWIW this change feels like it goes against the stated default behaviour in the README of:

The default behavior of this bot is not to police all commit messages, ...

and,

By default, only the PR title OR at least one commit message needs to have semantic prefix

kceb commented 4 years ago

We have a bunch of repos and are in the process of disabling it in each repo as a required check (tedious). Is there a quick way to get around this/revert the most recent changes?

Nadock commented 4 years ago

@kceb You've got a choice, either:

zeke commented 4 years ago

Hey thanks for reporting and sorry for the confusion. I think the README should probably updated with parenthetical about how single-commit PRs are an exception, because GitHub doesn't squash those.

zeke commented 4 years ago

This fix might be helpful to some of you, depending on your configuration: https://github.com/zeke/semantic-pull-requests/pull/109

mrchief commented 4 years ago

Came here to open up an issue about this and found this issue - so let me add my thoughts here. The current behavior is the right behavior - for single commit PRs, Github uses the commit's title and not the PR title (why Github has this quirk is an open issue I think). So by blocking it, you're ensuring that your commits in default branch are always semantic.

That said, I wish this worked beyond isVanillaConfig case.

zeke commented 4 years ago

Thanks for chiming in @mrchief. Nice to have a bit of validation behind my intent with the changes I made in https://github.com/zeke/semantic-pull-requests/pull/105

I wish this worked beyond isVanillaConfig case.

isVanillaConfig was my way of treading lightly to avoid breaking the myriad potential configurations that users of this app might be using, but I would happily accept a pull request that adds more general support for this behavior without breaking other configurations.

BeyondEvil commented 3 years ago

We have this: Ensure all single-commit PRs have a title line that is semantic

Still getting the dreaded ❌ .

Should mention, we're using the eslint preset, so our semantic.yml looks like this:

types:
  - Fix
  - Update
  - New
  - Breaking
  - Docs
  - Build
  - Upgrade
  - Chore

But no dice.

gr2m commented 3 years ago

I was wondering if you could add an option that would add an empty commit in case a pull request is valid but the single commit message is not. The pull request would have two commits and the default message for the squash & commit merge strategy would be correctly set.

I fear however that creating even an empty commit requires write access to files, so that might be a blocker. I'll probably create a minimal single-purpose app that would only do this one thing if you cannot add it to semantic-pull-requests, it might be useful anyway, e.g. if I don't want to burden new contributors with the semantic commit message checks

oscard0m commented 3 years ago

I was wondering if you could add an option that would add an empty commit in case a pull request is valid but the single commit message is not. The pull request would have two commits and the default message for the squash & commit merge strategy would be correctly set.

I fear however that creating even an empty commit requires write access to files, so that might be a blocker. I'll probably create a minimal single-purpose app that would only do this one thing if you cannot add it to semantic-pull-requests, it might be useful anyway, e.g. if I don't want to burden new contributors with the semantic commit message checks

What about this approach used here for example:

https://github.com/amannn/action-semantic-pull-request/blob/db6e259b93f286e3416eef27aaae88935d16cf2e/src/index.js#L54-L61

Recognize when it's a single commit and treat it in a different way would be an option?

gr2m commented 3 years ago

A GitHub Action would be to slow. You often times want to change the pull request title and then merge a few seconds after that. A GitHub Action will take at least 20s. Ideally it would work within 2s

don-hover commented 3 years ago

@zeke Please see: https://github.com/zeke/semantic-pull-requests/pull/105#issuecomment-785975869

mrchief commented 3 years ago

A GitHub Action would be to slow. You often times want to change the pull request title and then merge a few seconds after that. A GitHub Action will take at least 20s. Ideally it would work within 2s

So amannn/action-semantic-pull-request is a node action and it's much faster. I see it finish within 5s most of the time. You can still spend a little time waiting for a runner to become available if you have lot of jobs going on at the same time but for most of folks, that should be a non-issue.

oscard0m commented 3 years ago

Just to do a follow up here. Squash Commit App was released a few days ago in case you want to give it a try. https://github.com/apps/squash-commit-app

zeke commented 3 years ago

https://github.com/squash-commit-app/squash-commit-app

Neat!

zeke commented 2 years ago

This issue is old and inactive so I'm going to close it. If anyone is still affected by this, let me know and I'll re-open it.