Skip to content
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: use regex pattern to valid message #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wu-cl
Copy link
Contributor

@wu-cl wu-cl commented Apr 13, 2023

No description provided.

@wu-cl
Copy link
Contributor Author

wu-cl commented Apr 13, 2023

The current matching rules are slightly different from conventional commits
And this change will conflict with #10

expect(isValidCommitMessage("fix(menus): menu must open on shortcut press")).toBe(true);
expect(isValidCommitMessage("🚧 fix(menus): menu must open on shortcut press")).toBe(true);
expect(isValidCommitMessage("🚧 fix(menus): menu must open on shortcut press")).toBe(false);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that adding emojis to commit messages should be fine as long as the rest of the commit messages adheres to the conventional commits semantics.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this plugin should match the spec as close as possible and I'm not sure emojis are supported: https://www.conventionalcommits.org/en/v1.0.0/#specification

.replace(/[^a-z]/g, ""); // Only leave [a-z] characters.

return availableTypes.includes(possiblyValidCommitType);
let pattern = /^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: \S+([\s\S]*)/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather the DEFAULT_COMMIT_TYPES be as it is and use

commitPrefixes = DEFAULT_COMMIT_TYPES.join("|");

to build the pattern. This makes it easier to extend the list of valid commit prefixes and improves readability.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth noting this change is also not backwards compatible as the allowed types are no longer supported. I don't think this is a desirable change and if it were to proceed the PR should updated to feat! as it's a breaking change.

I believe it's possible to update the regex without removing the existing capabilities.

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.

3 participants