Skip to content

fix: Refuse to push on pull_request event #36

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

Merged

Conversation

Kurt-von-Laven
Copy link
Contributor

@Kurt-von-Laven Kurt-von-Laven commented May 11, 2022

Avoid merging pull requests, which is unlikely to be the intended purpose of using this action.

Don't pull or push with nothing to push. Check whether there are any commits to push by comparing the project version before and after bumping.

Print error message to stderr.

Fixes #29.

@Kurt-von-Laven Kurt-von-Laven force-pushed the dont-merge-pull-requests branch 2 times, most recently from 08f162f to aa39608 Compare May 19, 2022 18:44
@Kurt-von-Laven Kurt-von-Laven changed the title fix: Don't pull or push with nothing to push fix: Refuse to push on pull_request event May 19, 2022
@woile
Copy link
Member

woile commented Jul 6, 2022

I think this is a great addition, but we also cannot predict what people use this action for. I like having this as the default behavior, but could you add a setting to override it? Thanks!

Check whether there are any commits to push by comparing the project
version before and after bumping.
@Kurt-von-Laven Kurt-von-Laven force-pushed the dont-merge-pull-requests branch from aa39608 to 873b495 Compare July 6, 2022 20:38
Avoid merging pull requests, which is unlikely to be the intended
purpose of using this action.
@Kurt-von-Laven Kurt-von-Laven force-pushed the dont-merge-pull-requests branch from 873b495 to cb1c729 Compare July 6, 2022 20:44
@Kurt-von-Laven
Copy link
Contributor Author

Thank you for the feedback; I added a merge input that allows the new default behavior to be overridden. Let me know if a different parameter name would be preferable.

@woile
Copy link
Member

woile commented Jul 7, 2022

Fantastic, I would make it true by default. Just to be clear I think this is a good check. I'm just worried if someone is already using it in a different way, that's why I would like to have the option to skip it. But specially for newcomers I think it should be on by default 👍🏻 thanks again!

@Kurt-von-Laven
Copy link
Contributor Author

If I make the default value of merge true, I believe that would turn this feature off by default, retaining the existing behavior unless false is explicitly passed.

@woile
Copy link
Member

woile commented Jul 7, 2022

Oh sorry, didn't read properly. Let mergee 🎉

@woile woile merged commit 0d10fe0 into commitizen-tools:master Jul 7, 2022
@Kurt-von-Laven Kurt-von-Laven deleted the dont-merge-pull-requests branch July 7, 2022 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't Merge Pull Requests
2 participants