fix(changelog): handle custom tag_format in changelog generation #995

grahamhar · 2024-02-28T20:51:46Z

When the tag_format does not follow the allowed schemas patterns then changlog generation fails.

Description

I am working in a mono repo with multiple .cz.toml configs (one per component) using tag_format to create tags for each component so they can be released independent of each other. When trying to generate the changelog for each component errors are generated see #845.

I was unsure how to add a test case for this if you have ideas please let me know and I will be happy to add.

Checklist

Add test cases to all the changes you introduce
Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
Test the changes on the local machine manually
Update the documentation for the changes

Expected behavior

changelogs will now be generated correctly when running cz bump --changelog

Steps to Test This Pull Request

codecov · 2024-02-28T20:54:10Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.60%. Comparing base (120d514) to head (7400023).
Report is 350 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #995      +/-   ##
==========================================
+ Coverage   97.33%   97.60%   +0.26%     
==========================================
  Files          42       55      +13     
  Lines        2104     2545     +441     
==========================================
+ Hits         2048     2484     +436     
- Misses         56       61       +5

Flag	Coverage Δ
unittests	`97.60% <100.00%> (+0.26%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

woile · 2024-03-01T06:38:26Z

You can add tests similar to this:
https://github.com/commitizen-tools/commitizen/blob/master/tests/commands/test_changelog_command.py#L1457-L1491
where you can also configure the toml file and simulate a user

grahamhar · 2024-03-02T14:06:58Z

I have taken a stab at the test case.

Lowaiz · 2024-03-22T11:04:25Z

commitizen/commands/changelog.py

-                latest_tag_version: str = bump.normalize_tag(
-                    changelog_meta.latest_version,
-                    tag_format=self.tag_format,
-                    scheme=self.scheme,
-                )
                start_rev = self._find_incremental_rev(
-                    strip_local_version(latest_tag_version), tags
+                    strip_local_version(changelog_meta.latest_version), tags
                )
-


Hello,
Removing this logic is breaking the search for custom tags (with a tag-format like example-${version}).
The first bump (with --changelog option) will have no problem and it will create the CHANGELOG.md file but any bump afterward will result in an error: commitizen.exceptions.NoRevisionError: No tag found to do an incremental changelog.

This is due to the _find_incremental_rev function that look at similarity between a given tag and the tags list. But without the normalization, it calculates similarity between a stripped tag (without any format so like 0.1.1) and the formatted tag (like example-0.1.0) resulting in a similarity below threshold.

So, except if you have another edge case justifying this removal, I think it should be kept.

Hi,

Thanks for the feedback, I'll put this to a draft status and try to find a solution that doesn't break the current way of working. My gut feeling is it might need an additional flag maybe something like --strict-tag-matching what are your thoughts on taking that approach?

I guess the fact you found that error might mean there is a missing test case, I will try adding that first to replicate the error with my changes so I have something to validate against.

I used your version with a revert on the selected part (here), and it seems to works pretty damn well for me, that's why I was asking for the "why" of the change.
I was able to bump and generate for multiple cz.toml file and multiple tag-format (in a monorepo with 3 sub-packages).

The normalizing part was introduced by that PR, exactly fixing the error I got.

I really think that you just need to keep the normalizing part, and we should be OK.

Hi,

Thanks for clarifying I misunderstood your point.

I added a test that shows the failure you describe with my approach.

However, when I update my test to then do a second commit/bump then I start to get a failure as the changelog_meta.latest_version for my scenario where the custom tag values comes after the version number is returned as 0.2.0custom (str) which then causes an Invalid version error from the call to scheme in the bump.normalize_tag function.

I found that the change log is written with tag_format in the title for each section but then parse_version_from_title uses the parser from the version schema which means that when the tag format has a suffix rather than a prefix the issue occurs.

I will try to investigate further but would value your thoughts.

Is there anything that needs clarification for this one? (I guess not?) or should I start reviewing?

I just want to check the way I have handled the different possible tag_formats in markdown.py seems OK. If it is I will make the appropriate changes to the other formats taking the same approach. Once I have completed that it will be ready for review

I don't see any major flaws at first glance, but I will try to take a deeper look this weekend.

Sorry it took a while for me to get this finished off. Ready for review when you have time.

Thanks! I was not able to review it last week. 😞 Let's see whether I can at least check a portion of it this week. 💪

noirbizarre

Seems good but:

TAG_FORMAT_REGEXS should be factorized and declared once
the new ${} syntax should be documented

noirbizarre · 2024-04-17T15:25:43Z

commitizen/changelog_formats/base.py

+        TAG_FORMAT_REGEXS = {
+            "$version": version_regex,
+            "$major": r"(?P<major>\d+)",
+            "$minor": r"(?P<minor>\d+)",
+            "$patch": r"(?P<patch>\d+)",
+            "$prerelease": r"(?P<prerelease>\w+\d+)?",
+            "$devrelease": r"(?P<devrelease>\.dev\d+)?",
+            "${version}": version_regex,
+            "${major}": r"(?P<major>\d+)",
+            "${minor}": r"(?P<minor>\d+)",
+            "${patch}": r"(?P<patch>\d+)",
+            "${prerelease}": r"(?P<prerelease>\w+\d+)?",
+            "${devrelease}": r"(?P<devrelease>\.dev\d+)?",
+        }


Those are exactly the same as in changelog:get_version_tag so I suggest to factorize this into a public factory, something like:

TAG_FORMAT_REGEXS = { "$major": r"(?P<major>\d+)", "$minor": r"(?P<minor>\d+)", "$patch": r"(?P<patch>\d+)", "$prerelease": r"(?P<prerelease>\w+\d+)?", "$devrelease": r"(?P<devrelease>\.dev\d+)?", "${version}": version_regex, "${major}": r"(?P<major>\d+)", "${minor}": r"(?P<minor>\d+)", "${patch}": r"(?P<patch>\d+)", "${prerelease}": r"(?P<prerelease>\w+\d+)?", "${devrelease}": r"(?P<devrelease>\.dev\d+)?", } def tag_format_regexps_for(version: Pattern) -> dict[str, Pattern]: return { "$version": version, "${version}": version, **TAG_FORMAT_REGEXS }

This way:

single source of thrust

custom format implementations can reuse TAG_FORMAT_REGEXS and tag_format_regexps_for

noirbizarre · 2024-04-17T15:29:11Z

commitizen/providers/scm_provider.py

+        "${version}": r"(?P<version>.+)",
+        "${major}": r"(?P<major>\d+)",
+        "${minor}": r"(?P<minor>\d+)",
+        "${patch}": r"(?P<patch>\d+)",
+        "${prerelease}": r"(?P<prerelease>\w+\d+)?",
+        "${devrelease}": r"(?P<devrelease>\.dev\d+)?",


This is a new accepted syntax.
Documentation and conventional commit message should reflect that

noirbizarre · 2024-04-17T15:31:10Z

commitizen/providers/scm_provider.py

+        "${minor}": r"(?P<minor>\d+)",
+        "${patch}": r"(?P<patch>\d+)",
+        "${prerelease}": r"(?P<prerelease>\w+\d+)?",
+        "${devrelease}": r"(?P<devrelease>\.dev\d+)?",


Very similar to the 2 previous factorizable TAG_FORMAT_REGEXS so I think it should be factorized too,

grahamhar · 2024-04-20T10:28:35Z

Seems good but:

TAG_FORMAT_REGEXS should be factorized and declared once

the new ${} syntax should be documented

I think I have implemented the suggestions

commitizen/changelog_formats/asciidoc.py

commitizen/changelog_formats/markdown.py

commitizen/providers/scm_provider.py

commitizen/changelog_formats/restructuredtext.py

commitizen/changelog_formats/textile.py

Lee-W · 2024-04-21T07:29:11Z

commitizen/defaults.py

@@ -133,3 +133,20 @@ class Settings(TypedDict, total=False):
 )
 change_type_order = ["BREAKING CHANGE", "Feat", "Fix", "Refactor", "Perf"]
 bump_message = "bump: version $current_version → $new_version"
+
+
+def get_tag_regexes(version_regex: str) -> dict[str | Any, str | Any]:


When will the key return Any?

Lee-W

Sorry for taking so long to review. Most of my comments are just nitpicks. It would be great if we could fix them, but I'm good with it if we cannot. As this is somewhat a larger change, I think it would be better if we can have @woile and @noirbizarre take a look as well 🙂

Lee-W · 2024-05-20T14:46:12Z

commitizen/defaults.py

+        "${minor}": r"(?P<minor>\d+)",
+        "${patch}": r"(?P<patch>\d+)",
+        "${prerelease}": r"(?P<prerelease>\w+\d+)?",
+        "${devrelease}": r"(?P<devrelease>\.dev\d+)?",


nitpick: not sure we can deduplicate $.* and ${.*} 🤔 Non-blocker for this PR

commitizen/providers/scm_provider.py

docs/bump.md

docs/tutorials/monorepo_guidance.md

tests/commands/test_changelog_command.py

tests/test_changelog_format_asciidoc.py

Lee-W · 2024-05-20T18:32:02Z

tests/test_changelog_format_markdown.py

+        pytest.param("${version}-example", "1.0.0-example", "1.0.0"),
+        pytest.param("${version}example", "1.0.0example", "1.0.0"),
+        pytest.param("example${version}", "example1.0.0", "1.0.0"),
+        pytest.param("example-${version}", "example-1.0.0", "1.0.0"),
+        pytest.param("example-${major}-${minor}-${patch}", "example-1-0-0", "1.0.0"),
+        pytest.param("example-${major}-${minor}", "example-1-0-0", None),
+        pytest.param(
+            "${major}-${minor}-${patch}-${prerelease}-example",
+            "1-0-0-rc1-example",
+            "1.0.0-rc1",
+        ),
+        pytest.param(
+            "${major}-${minor}-${patch}-${prerelease}-example",
+            "1-0-0-a1-example",
+            "1.0.0-a1",
+        ),
+        pytest.param(
+            "${major}-${minor}-${patch}-${prerelease}${devrelease}-example",
+            "1-0-0-a1.dev1-example",
+            "1.0.0-a1.dev1",
+        ),


feel like we can make it reuseable somewhere 🤔

Lee-W · 2024-05-20T18:36:37Z

We probably need to resolve the conflict as well 👀 . We're now using screenshots as part of our doc. you can generate the latest screenshot through poetry run python scripts/gen_cli_help_screenshots.py

woile · 2024-05-21T05:59:08Z

docs/tutorials/monorepo_guidance.md

+      .cz.toml
+```
+
+Each component will have its own changelog, commits will need to use scopes so only relevant commits are included in the


What if you are not using conventional commits and you don't have scopes?

Sorry, I guess I made an assumption here that everyone uses them. Should I just reword this to reflect that assumption?

grahamhar · 2024-05-22T04:34:29Z

Sorry for taking so long to review. Most of my comments are just nitpicks. It would be great if we could fix them, but I'm good with it if we cannot. As this is somewhat a larger change, I think it would be better if we can have @woile and @noirbizarre take a look as well 🙂

No worries on the time taken, I will address all the points but might take me a week due to other commitments

When the tag_format does not follow the allowed schemas patterns then changlog generation fails.

Co-authored-by: Wei Lee <weilee.rx@gmail.com>

grahamhar · 2024-05-29T19:44:45Z

We probably need to resolve the conflict as well 👀 . We're now using screenshots as part of our doc. you can generate the latest screenshot through poetry run python scripts/gen_cli_help_screenshots.py

Hi @Lee-W,

I have attempted to address the comments, please validate when you have time.

When attempting to run the gen_cli_help_screenshots.py script requested above I just get this output which appears to just be given usage errors of cz

I'm not sure if you are already aware, but with the push hook in place I got an error for a commit not in my PR

commit validation: failed!
please enter a commit message in the commitizen format.
commit "37522866e4788deb12b2ef1c426662400b0ebac8": "docs(cli/screenshots) update CLI screenshots"
pattern: (?s)(build|ci|docs|feat|fix|perf|refactor|style|test|chore|revert|bump)(\(\S+\))?!?:( [^\n\r]+)((\n\n.*)|(\s*))?$

Lee-W · 2024-05-30T02:11:13Z

When attempting to run the gen_cli_help_screenshots.py script requested above I just get this output which appears to just be given usage errors of cz

The screenshot doesn't look like an error to me 🤔 so basically, this is a command we use to generate the latest cz help message. so if the screenshots are generated, I think we are good 🙂

I'm not sure if you are already aware, but with the push hook in place I got an error for a commit not in my PR

Even if you try with rebase? There was one commit with an ill-formatted message. But our hook should only check the commits in this branch, I think 🤔 It Looks like the CI is passing now. Is this still an issue?

grahamhar · 2024-05-31T15:10:34Z

When attempting to run the gen_cli_help_screenshots.py script requested above I just get this output which appears to just be given usage errors of cz

The screenshot doesn't look like an error to me 🤔 so basically, this is a command we use to generate the latest cz help message. so if the screenshots are generated, I think we are good 🙂

I'm not sure if you are already aware, but with the push hook in place I got an error for a commit not in my PR

Even if you try with rebase? There was one commit with an ill-formatted message. But our hook should only check the commits in this branch, I think 🤔 It Looks like the CI is passing now. Is this still an issue?

Maybe my PR didn't need any changes to the generated images, possibly just confusion on my side.

I have just made sure my fork is updated and as I have no changes I just tried running the hook

 pre-commit run --hook-stage pre-push                      
Check hooks apply to the repository.......................(no files to check)Skipped
Check for useless excludes................................(no files to check)Skipped
check vcs permalinks......................................(no files to check)Skipped
fix end of files..........................................(no files to check)Skipped
trim trailing whitespace..................................(no files to check)Skipped
debug statements (python).................................(no files to check)Skipped
don't commit to branch........................................................Passed
check for merge conflicts.................................(no files to check)Skipped
check toml................................................(no files to check)Skipped
check yaml................................................(no files to check)Skipped
detect private key........................................(no files to check)Skipped
blacken-docs..............................................(no files to check)Skipped
Run codespell to check for common misspellings in files...(no files to check)Skipped
commitizen check branch.......................................................Failed
- hook id: commitizen-branch
- exit code: 23

fatal: ambiguous argument 'origin/HEAD..HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'



format....................................................(no files to check)Skipped
linter and test...........................................(no files to check)Skipped

Doing some further investigation my fork doesn't have origin/HEAD set. After running

git remote set-head origin master

The hook now works :)

Lee-W · 2024-05-31T15:32:50Z

The hook now works :)

Niiiiiiice! I will probably not be around for half a month, but I will try to take a look after I'm back from traveling. Thanks for actively helping out.

grahamhar requested review from woile, Lee-W and noirbizarre as code owners February 28, 2024 20:51

github-actions bot added the pr-status: wait-for-review label Feb 28, 2024

grahamhar mentioned this pull request Feb 29, 2024

Add support for scoped changelog generation: cz changelog --scope myscope #530

Open

Lowaiz reviewed Mar 22, 2024

View reviewed changes

grahamhar marked this pull request as draft March 26, 2024 13:26

grahamhar force-pushed the regex-tags branch from a7cea66 to fcfd4dc Compare March 26, 2024 18:25

Lee-W added pr-status: wait-for-response and removed pr-status: wait-for-review labels Mar 30, 2024

grahamhar force-pushed the regex-tags branch from fcfd4dc to d6b0745 Compare April 1, 2024 17:43

github-actions bot added the pr-status: wait-for-review label Apr 1, 2024

grahamhar force-pushed the regex-tags branch 8 times, most recently from 0cc5cef to 7d74d80 Compare April 7, 2024 13:42

grahamhar marked this pull request as ready for review April 7, 2024 13:50

Lee-W removed the pr-status: wait-for-response label Apr 8, 2024

noirbizarre requested changes Apr 17, 2024

View reviewed changes

Lee-W requested a review from noirbizarre April 21, 2024 06:53

Lee-W reviewed Apr 21, 2024

View reviewed changes

Lee-W added the pr-status: wait-for-modification label Apr 21, 2024

Lee-W removed the pr-status: wait-for-review label Apr 21, 2024

noirbizarre mentioned this pull request Apr 24, 2024

Next major release ? #1073

Open

3 tasks

github-actions bot added the pr-status: wait-for-review label Apr 27, 2024

grahamhar force-pushed the regex-tags branch from 7e7e848 to d5dcf4d Compare April 27, 2024 14:34

Lee-W removed the pr-status: wait-for-modification label May 20, 2024

Lee-W reviewed May 20, 2024

View reviewed changes

Lee-W assigned Lee-W, noirbizarre and woile and unassigned Lee-W May 20, 2024

Lee-W added the pr-status: wait-for-modification label May 20, 2024

woile reviewed May 21, 2024

View reviewed changes

Lee-W unassigned woile May 22, 2024

grahamhar and others added 6 commits May 29, 2024 19:27

fix(changelog): handle custom tag_format in changelog generation

89e00d7

When the tag_format does not follow the allowed schemas patterns then changlog generation fails.

test(changelog): handle custom tag_format in changelog generation

eb6b98f

fix(changelog): Handle tag format without version pattern

cd562aa

fix(changelog): Factorized TAG_FORMAT_REGEXES

fc69622

docs(bump): Document the use of tag_format variables with curly brackets

9bdeba2

refactor: Use format strings

7400023

Co-authored-by: Wei Lee <weilee.rx@gmail.com>

grahamhar force-pushed the regex-tags branch from 174d809 to 7400023 Compare May 29, 2024 19:33

Lee-W added pr-status: wait-for-response and removed pr-status: wait-for-modification labels May 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(changelog): handle custom tag_format in changelog generation #995

fix(changelog): handle custom tag_format in changelog generation #995

grahamhar commented Feb 28, 2024 •

edited by Lee-W

Loading

codecov bot commented Feb 28, 2024 •

edited

Loading

woile commented Mar 1, 2024

grahamhar commented Mar 2, 2024

Lowaiz Mar 22, 2024 •

edited

Loading

grahamhar Mar 26, 2024

Lowaiz Mar 26, 2024

grahamhar Mar 26, 2024

grahamhar Mar 26, 2024

Lee-W Apr 2, 2024

grahamhar Apr 2, 2024

Lee-W Apr 2, 2024 •

edited

Loading

grahamhar Apr 7, 2024

Lee-W Apr 8, 2024

noirbizarre left a comment

noirbizarre Apr 17, 2024

noirbizarre Apr 17, 2024

noirbizarre Apr 17, 2024

grahamhar commented Apr 20, 2024

Lee-W Apr 21, 2024

grahamhar Apr 27, 2024

Lee-W left a comment

Lee-W May 20, 2024

Lee-W May 20, 2024

Lee-W commented May 20, 2024

woile May 21, 2024

grahamhar May 22, 2024

grahamhar commented May 22, 2024

grahamhar commented May 29, 2024

Lee-W commented May 30, 2024

grahamhar commented May 31, 2024

Lee-W commented May 31, 2024

fix(changelog): handle custom tag_format in changelog generation #995

Are you sure you want to change the base?

fix(changelog): handle custom tag_format in changelog generation #995

Conversation

grahamhar commented Feb 28, 2024 • edited by Lee-W Loading

Description

Checklist

Expected behavior

Steps to Test This Pull Request

codecov bot commented Feb 28, 2024 • edited Loading

Codecov Report

woile commented Mar 1, 2024

grahamhar commented Mar 2, 2024

Lowaiz Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lee-W Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noirbizarre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grahamhar commented Apr 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lee-W left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lee-W commented May 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grahamhar commented May 22, 2024

grahamhar commented May 29, 2024

Lee-W commented May 30, 2024

grahamhar commented May 31, 2024

Lee-W commented May 31, 2024

grahamhar commented Feb 28, 2024 •

edited by Lee-W

Loading

codecov bot commented Feb 28, 2024 •

edited

Loading

Lowaiz Mar 22, 2024 •

edited

Loading

Lee-W Apr 2, 2024 •

edited

Loading