commitizen-tools / commitizen

Create committing rules for projects :rocket: auto bump versions :arrow_up: and auto changelog generation :open_file_folder:
https://commitizen-tools.github.io/commitizen/
MIT License
2.55k stars 266 forks source link

Use named capture group in bump_pattern to enable stricter check #129

Open Lee-W opened 4 years ago

Lee-W commented 4 years ago

Goal make regular expression pattern stricter so that we won't accidentally match things we don't need

Description In commitizen/cz/conventional_commits/conventional_commits.py#L33 on command-changelog branch, I use named capture group so that we could use a stricter regular expression like .*\n\nBREAKING CHANGE. The benefit of it is that we don't have to break the whole commit message into lines like commitizem/bump.py#L31. It can also avoid bump or generate changelog based on commit message like fix --- it does not follow the rule but still match the pattern. Another thought on this topic is that we probably merge the bump_map and bump_pattern into one some data class to store name(e.g., break), pattern(e.g., .*\n\nBREAKING CHANGE), behavior(e.g., PATCH).

woile commented 4 years ago

Not sure how to follow with this one. do we still need it?

Would the dataclass be internal or it should be provided for templating?

Lee-W commented 4 years ago

The basic idea of the dataclass part is to make moving from dict to dataclass which might be clear.

The main idea of this issue is to use named capture group to make the regular expression stricter like how you implement commit_parser.

# now
bump_pattern = r"^(BREAKING[\-\ ]CHANGE|feat|fix|refactor|perf)(\(.+\))?(!)?"

# named capture group
bump_pattern = r"(?P<MAJOR>^.*\n\nBREAKING[-]CHANGE.*|)|(?P<MINOR>^feat.*)|?(P<PATCH>^fix.*|^perf.*|^refactor.*)"
woile commented 4 years ago

Oh, I see, the find_increment would have to be completely refactored.

Still I see some complications, for conventional commits how would you capture BREAKING CHANGE and ! as breaking chagnes with a named group? I've tried a while ago with little success haha

Regarding the dataclasses I'd need an example to understand it better, for me a dict is usually clearer than anything, and can be easily converted to configuration if it's kept simple.

Lee-W commented 4 years ago

Still I see some complications, for conventional commits how would you capture BREAKING CHANGE and ! as breaking chagnes with a named group? I've tried a while ago with little success haha

Things like (?P<MAJOR>^.*\n\nBREAKING[-]CHANGE.*|)|(?P<MINOR>^feat.*), but not yet testesd.

Regarding the dataclasses I'd need an example to understand it better, for me a dict is usually clearer than anything, and can be easily converted to configuration if it's kept simple.

I'm working on this refactoring in #203 . (I've not yet get to the dataclass part.) IMO, dataclass is a stricter solution and less error-prone. It explicitly indicates the type of each configuration. I'll give you an example once I implement a prototype

woile commented 4 years ago

I mean like these cases, how would we parse them? They both introduce breaking changes, and they'd use MAJOR as a group variable, right

refactor!: drop support for Python 2.7
feat: allow provided config object to extend other configs

BREAKING CHANGE: `extends` key in config file is now used for extending other config files
Lee-W commented 4 years ago

It seems we do not need to parse the message when we bump the project version. All we want to know is which version (i.e. MAJOR, MINOR, PATCH) to bump. What we need to know if whether these types (e.g., MAJOR, MINOR, PATCH) of commits exist.

woile commented 1 year ago

Any update on this?

Lee-W commented 1 year ago

not at this moment 😢