commitizen icon indicating copy to clipboard operation
commitizen copied to clipboard

fix(changelog): handle custom tag_format in changelog generation

Open grahamhar opened this issue 11 months ago • 11 comments

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

  • [x] Add test cases to all the changes you introduce
  • [x] Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • [x] Test the changes on the local machine manually
  • [x] 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

grahamhar avatar Feb 28 '24 20:02 grahamhar

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.63%. Comparing base (120d514) to head (3d65861). Report is 424 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #995      +/-   ##
==========================================
+ Coverage   97.33%   97.63%   +0.29%     
==========================================
  Files          42       55      +13     
  Lines        2104     2575     +471     
==========================================
+ Hits         2048     2514     +466     
- Misses         56       61       +5     
Flag Coverage Δ
unittests 97.63% <100.00%> (+0.29%) :arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 28 '24 20:02 codecov[bot]

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

woile avatar Mar 01 '24 06:03 woile

I have taken a stab at the test case.

grahamhar avatar Mar 02 '24 14:03 grahamhar

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

grahamhar avatar Apr 20 '24 10:04 grahamhar

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

Lee-W avatar May 20 '24 18:05 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 🙂

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

grahamhar avatar May 22 '24 04:05 grahamhar

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

image

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*))?$

grahamhar avatar May 29 '24 19:05 grahamhar

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?

Lee-W avatar May 30 '24 02:05 Lee-W

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 :)

grahamhar avatar May 31 '24 15:05 grahamhar

The hook now works :)

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.

Lee-W avatar May 31 '24 15:05 Lee-W

Sorry for the looong absence. I'll take the time to do a final review by the end of the week (as this one is a big one, and I need to reread and remember all what have been said)

noirbizarre avatar Aug 20 '24 23:08 noirbizarre

This could potentially fix #1149 right?

woile avatar Sep 12 '24 12:09 woile