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

feat: add config option to validate any commit instead of all commits #71

Closed clacan closed 5 years ago

clacan commented 5 years ago

My organization would like to be able to validate PRs using PR title and any (but not every) commit. This adds an anyCommit config option that makes this possible (when used in conjunction with titleAndCommits: true or commitsOnly: true).

I believe this addresses #41

View rendered README.md

zeke commented 5 years ago

Hey @ccangialose! Thanks for submitting this PR and apologies for the delay reviewing it.

I will start off by saying that I am willing to merge this PR to support your organization's use case. You've added new tests and the existing tests continue to pass, so it appears this new functionality will work.

I will add though that I think the configuration options for this service should be reconfigured somehow. Currently we have these options: titleOnly, commitsOnly, titleAndCommits, and soon anyCommit. When considering all the possible combinations in which these options can be used, it's a bit confusing and seems inviting to bugs in the code.

Thinking out loud, what if we moved to something like this:

Prop Allowable Values Default
validateTitle true or false undefined
validateCommits 'all' or 'any' or false undefined

In the case where both validateTitle and validateCommits are undefined, then the default behavior would be to require at least one semantic commit OR a semantic title.

clacan commented 5 years ago

Hi @zeke! Thank you for the review.

I agree, and I think your proposed reconfiguration is a great, clean solution. If you would like, I would happily put some time into attempting to implement it and contribute something that provides greater benefit to the project.

zeke commented 5 years ago

Thanks for your willingness to help out, @ccangialose!

One thing that would be tricky about making these changes would be there are already lots of users of this service, and if we change the config, then their integrations will no longer work as expected... We'd probably need to update this bot to open issues on repos when their semantic.yml configuration is outdated.

Maybe we should ship this PR to unblock your organization, and then follow up on a refactor in a separate PR. Does that sound good?

clacan commented 5 years ago

@zeke That sounds great! Thank you.