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.59%. Comparing base (120d514) to head (d5dcf4d).
Report is 327 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #995      +/-   ##
==========================================
+ Coverage   97.33%   97.59%   +0.25%     
==========================================
  Files          42       55      +13     
  Lines        2104     2536     +432     
==========================================
+ Hits         2048     2475     +427     
- Misses         56       61       +5

Flag	Coverage Δ
unittests	`97.59% <100.00%> (+0.25%)`	⬆️

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. 💪

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

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?

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

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

Lee-W · 2024-05-20T14:51:10Z

docs/bump.md

+| `$version`      | full generated version                      |
+| `$major`        | MAJOR increment                             |
+| `$minor`        | MINOR increment                             |
+| `$patch`        | PATCH increment                             |
+| `$prerelease`   | Prerelease (alpha, beta, release candidate) |
+| `$devrelease`   | Development release                         |
+| `${version}`    | full generated version                      |
+| `${major}`      | MAJOR increment                             |
+| `${minor}`      | MINOR increment                             |
+| `${patch}`      | PATCH increment                             |
+| `${prerelease}` | Prerelease (alpha, beta, release candidate) |
+| `${devrelease}` | Development release                         |


Suggested change

| `$version` | full generated version |

| `$major` | MAJOR increment |

| `$minor` | MINOR increment |

| `$patch` | PATCH increment |

| `$prerelease` | Prerelease (alpha, beta, release candidate) |

| `$devrelease` | Development release |

| `${version}` | full generated version |

| `${major}` | MAJOR increment |

| `${minor}` | MINOR increment |

| `${patch}` | PATCH increment |

| `${prerelease}` | Prerelease (alpha, beta, release candidate) |

| `${devrelease}` | Development release |

| `$version`, `${version}` | full generated version |

| `$major`, `${major}` | MAJOR increment |

| `$minor`, `${minor}` | MINOR increment |

| `$patch`, `${patch}` | PATCH increment |

| `$prerelease`, `${prerelease}` | Prerelease (alpha, beta, release candidate) |

| `$devrelease`, ${devrelease}` | Development release |

I think it might make it easier to read.

Lee-W · 2024-05-20T14:54:47Z

docs/tutorials/monorepo_guidance.md

+library-a
+  .cz.toml
+library-b
+  .cz.toml


Suggested change

library-a

.cz.toml

library-b

.cz.toml

.

├── library-b

│ └── .cz.toml

└── library-z

└── .cz.toml

Lee-W · 2024-05-20T14:55:08Z

docs/tutorials/monorepo_guidance.md

+src
+    library-b
+      .cz.toml
+    library-z
+      .cz.toml


Suggested change

src

library-b

.cz.toml

library-z

.cz.toml

src

├── library-b

│ └── .cz.toml

└── library-z

└── .cz.toml

Lee-W · 2024-05-20T18:29:51Z

tests/commands/test_changelog_command.py

+    git.tag("random0.2.0")
+    testargs = ["cz", "bump", "--changelog", "--yes"]
+    mocker.patch.object(sys, "argv", testargs)
+    cli.main()


Suggested change

cli.main()

# bump to 0.2.0custom

cli.main()

Read a few times to understand fully. would be great if we can have comments here 🙂

Lee-W · 2024-05-20T18:30:03Z

tests/commands/test_changelog_command.py

+    cli.main()
+    wait_for_tag()
+    create_file_and_commit("feat: another new file")
+    cli.main()


Suggested change

cli.main()

# bump to 0.3.0custom

cli.main()

Lee-W · 2024-05-20T18:30:59Z

tests/test_changelog_format_asciidoc.py

+
+
+@pytest.mark.parametrize(
+    "format_with_tags, tag_string, expected, ",


Suggested change

"format_with_tags, tag_string, expected, ",

"format_with_tags, tag_string, expected",

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

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 4 times, most recently from 5a9da67 to 21c46b1 Compare April 7, 2024 12:41

grahamhar added 2 commits April 7, 2024 13:42

fix(changelog): handle custom tag_format in changelog generation

d5f127d

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

test(changelog): handle custom tag_format in changelog generation

0a89510

grahamhar force-pushed the regex-tags branch 3 times, most recently from d1d6611 to 0cc5cef Compare April 7, 2024 13:22

fix(changelog): Handle tag format without version pattern

7d74d80

grahamhar force-pushed the regex-tags branch 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

fix(changelog): Factorized TAG_FORMAT_REGEXES

8d08840

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

c19cd1d

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

Lee-W reviewed Apr 21, 2024

View reviewed changes

Lee-W added pr-status: wait-for-modification and removed pr-status: wait-for-review labels 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

refactor: Use format strings

d5dcf4d

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

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

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

codecov bot commented Feb 28, 2024 •

edited

woile commented Mar 1, 2024

grahamhar commented Mar 2, 2024

Lowaiz Mar 22, 2024 •

edited

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

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 May 20, 2024

Lee-W May 20, 2024

Lee-W May 20, 2024

Lee-W May 20, 2024

Lee-W May 20, 2024

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



		@pytest.mark.parametrize(
		"format_with_tags, tag_string, expected, ",

	"format_with_tags, tag_string, expected, ",
	"format_with_tags, tag_string, expected",

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

Description

Checklist

Expected behavior

Steps to Test This Pull Request

codecov bot commented Feb 28, 2024 • edited

Codecov Report

woile commented Mar 1, 2024

grahamhar commented Mar 2, 2024

Lowaiz Mar 22, 2024 • edited

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

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

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 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 Feb 28, 2024 •

edited by Lee-W

codecov bot commented Feb 28, 2024 •

edited

Lowaiz Mar 22, 2024 •

edited

Lee-W Apr 2, 2024 •

edited