-
-
Notifications
You must be signed in to change notification settings - Fork 124
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
feat: add regex support to scope configuration #226
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey and thanks a lot for this PR!
This seems to be really well thought-out and I really appreciate that you made sure the feature is well tested.
I only left two minor comments but generally this looks very good, thanks again! 👏
README.md
Outdated
scopes: | | ||
core | ||
ui | ||
(?![A-Z])+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we maybe use the JIRA ID use case here? It seems like many users are interested in that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea
Do you think something somewhat specific like JIRA-\d+
would be sufficient? To draw the eye with the JIRA
part (that people can replace for their own project)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, sounds perfect! 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic PR, thanks a lot for your help! 🎉
🎉 This PR is included in version 5.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Reasoning
I would like to use this github action for PR validation in my project. It would be good as is, but we have adopted a convention where the
scope
includes a JIRA ticket ID, which in combination with github's auto-linking feature works wonders for readable changelogs. However, without regex support it's not possible to validate such scope.I have also noticed that and issue for this already exists: Closes #221
Changes
I've changed existing fields for scope configuration (
scopes
,disallowedScopes
) to be based around Regex, inspired by the solution for thetypes
proposed in #220My reasoning behind it was, that adding separate
scopePattern
input would be a bit confusing, and would add configurational complexity with then having to ignore/throw on e.g.scopes
being declared as well.To ensure backwards compatibilty I made it so the regex is auto-wrapped in
^ $
. This can be problematic if someone doesn't want to match it from start to end, but can be worked around. I've also included tests to ensure double wrapping is not an issue.I've included unit tests, tried to make it explicit with backwards compatibility.
Let me know if this makes sense :)
Demo PR
Kretolus#1