search

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

fix: disallow squashing of unsemantic single-commit PRs #105

Closed zeke closed 4 years ago

zeke commented 4 years ago

There's a subtle GitHub "bug" where squashed pull requests that only have one commit don't end up creating a squash commit.

This PR will make the GitHub app return a failing check in the following scenario:

Resolves https://github.com/zeke/semantic-pull-requests/issues/17

cc @rachmari @jmarlena @lucascosti @martin389

zeke commented 4 years ago

This should be auto-deployed to production soon.

lucascosti commented 4 years ago

Thanks, @zeke! 🎉

don-hover commented 3 years ago

@zeke We are using the GitHub app/action with this config:

# Always validate the PR title, and ignore the commits
titleOnly: true
# Always validate all commits, and ignore the PR title
commitsOnly: false
# Always validate the PR title AND all the commits
titleAndCommits: false
# Require at least one commit to be valid
# this is only relevant when using commitsOnly: true or titleAndCommits: true,
# which validate all commits by default
anyCommit: false
# Allow use of Merge commits (eg on github: "Merge branch 'master' into feature/ride-unicorns")
# this is only relevant when using commitsOnly: true (or titleAndCommits: true)
allowMergeCommits: true
# Allow use of Revert commits (eg on github: "Revert "feat: ride unicorns"")
# this is only relevant when using commitsOnly: true (or titleAndCommits: true)
allowRevertCommits: true

We also use dependabot, which creates single-commit PRs, and we use GitHub "Squash and Merge". With these dependabot single-commit PRs, we're seeing them be failed by the semantic-pull-requests check, even though the commit message and PR title are semantic (see screen shot). Could you explain why this is happening, and suggest a solution for this?

Thanks. Screen Shot 2021-02-25 at 10 13 41 AM

gr2m commented 3 years ago

dependabot is not a valid scope. Should be build. Did you configure it?

don-hover commented 3 years ago

dependabot is not a valid scope. Should be build. Did you configure it?

dependabot in this example is a type, not a scope. Yes, we've configured it. Our types config includes 'dependabot':

types:
  - feat
  - fix
  - docs
  - style
  - refactor
  - perf
  - test
  - build
  - ci
  - chore
  - revert
  - dependabot

The issue is not with the scope - the issue is specifically around the commit being checked and failing for a valid title..

don-hover commented 3 years ago

@zeke We are using the GitHub app/action with this config:

# Always validate the PR title, and ignore the commits
titleOnly: true
# Always validate all commits, and ignore the PR title
commitsOnly: false
# Always validate the PR title AND all the commits
titleAndCommits: false
# Require at least one commit to be valid
# this is only relevant when using commitsOnly: true or titleAndCommits: true,
# which validate all commits by default
anyCommit: false
# Allow use of Merge commits (eg on github: "Merge branch 'master' into feature/ride-unicorns")
# this is only relevant when using commitsOnly: true (or titleAndCommits: true)
allowMergeCommits: true
# Allow use of Revert commits (eg on github: "Revert "feat: ride unicorns"")
# this is only relevant when using commitsOnly: true (or titleAndCommits: true)
allowRevertCommits: true

We also use dependabot, which creates single-commit PRs, and we use GitHub "Squash and Merge". With these dependabot single-commit PRs, we're seeing them be failed by the semantic-pull-requests check, even though the commit message and PR title are semantic (see screen shot). Could you explain why this is happening, and suggest a solution for this?

Thanks. Screen Shot 2021-02-25 at 10 13 41 AM

@zeke Do you have anything that can help us with this issue? Currently we're using dependabot and every day have 5-15 dependabot PRs stacked up that are not being auto-merged, because of this issue with semantic-pull-requests action. We've done everything to configure it correctly, so need some explanation about why it's not working and how to resolve this.

zeke commented 3 years ago

Hi @don-hover,

I don't have time to dig into this specific case right now, but I can offer some suggestions:

  1. Try stripping down your config file to nothing, then incrementally adding the configuration you need, like the custom dependabot scope. External contributors have added new configuration options over the years, and now the codebase is a bit of a mess and bugs are more likely to come up as your config file grows. I still use this app, but without any custom configuration.
  2. If that doesn't work, try forking this repo and creating a test that reproduces your failure scenario. We can work together from there to fix it.
  3. Fork the repo, make any changes you need, host it yourself, and create a new GitHub App.
  4. Use a GitHub Action like https://github.com/amannn/action-semantic-pull-request instead of this app.

I hope that helps.

mowies commented 3 years ago

Was there a specific reason why this PR was only implemented for the vanilla/default config?