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.

Sign up for GitHub

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

Jump to bottom

feat(changelog): add merge-prerelease flag #703

Merged
merged 3 commits into from
Apr 17, 2023
Merged

feat(changelog): add merge-prerelease flag #703

merged 3 commits into from
Apr 17, 2023

Conversation

MoritzBoehme
Copy link
Contributor

@MoritzBoehme MoritzBoehme commented Apr 13, 2023 •
edited
Loading

Description

Added a --merge-prerelease flag and config option as described in #687.

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

Steps to Test This Pull Request

Additional context

The dependency codecov seems to be no longer available. I can try to fix it in this pull request, but could also create a new one, depending on what is preferred.
I have now to my own frustration fixed the codecov action in the second commit. This also closes #685

@MoritzBoehme MoritzBoehme force-pushed the merge-prerelease branch 6 times, most recently from ebf3f73 to d5f6722 Compare April 13, 2023 07:02
@MoritzBoehme MoritzBoehme marked this pull request as ready for review April 13, 2023 07:02
@MoritzBoehme MoritzBoehme changed the title feat(changelog): add merge_prereleases flag Draft: feat(changelog): add merge_prereleases flag Apr 13, 2023
@MoritzBoehme MoritzBoehme changed the title Draft: feat(changelog): add merge_prereleases flag feat(changelog): add merge_prereleases flag Apr 13, 2023
@codecov
Copy link

codecov bot commented Apr 13, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.12 🎉

Comparison is base (b5125cb) 97.98% compared to head (e34de55) 98.11%.

Additional details and impacted files 
@@            Coverage Diff             @@
##               v3     #703      +/-   ##
==========================================
+ Coverage   97.98%   98.11%   +0.12%     
==========================================
  Files          41       41              
  Lines        1835     1855      +20     
==========================================
+ Hits         1798     1820      +22     
+ Misses         37       35       -2
Flag Coverage Δ
unittests 98.11% <100.00%> (+0.12%) ⬆️

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

Impacted Files Coverage Δ
commitizen/cli.py 94.11% <ø> (ø)
commitizen/changelog.py 100.00% <100.00%> (+0.56%) ⬆️
commitizen/commands/changelog.py 98.88% <100.00%> (+0.02%) ⬆️
commitizen/defaults.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@MoritzBoehme MoritzBoehme force-pushed the merge-prerelease branch 3 times, most recently from cfbbfb5 to ab957a4 Compare April 13, 2023 08:23
@MoritzBoehme
Copy link
Contributor Author

MoritzBoehme commented Apr 13, 2023 •
edited
Loading

It seems the pre-commit hook prettier is failing for windows only (Seems as if it did succeed at least once there too). Weirdly it works for windows + python3.10 tough.
No idea, why this would be the case :/

@MoritzBoehme
Copy link
Contributor Author

MoritzBoehme commented Apr 13, 2023 •
edited
Loading

Found out another reason why test would randomly fail. It's caused by concurrent tests all accessing the same temporary file for the backup of the commit message. Changing the test script to run test independently seems to solve the issue.

@MoritzBoehme
Copy link
Contributor Author

@woile could you perhaps have a look at the pr? Do you maybe have some idea on how to keep prettier from failing?
Thanks for this awesome project btw =)

@MoritzBoehme MoritzBoehme changed the title feat(changelog): add merge_prereleases flag feat(changelog): add merge-prerelease flag Apr 16, 2023
woile
woile reviewed Apr 16, 2023
View reviewed changes
tests/test_changelog.py Outdated
gitcommits, tags, parser, changelog_pattern, merge_prerelease=True
)

assert tuple(tree) == COMMITS_TREE_MERGED_PRERELEASES
Copy link
Member

Choose a reason for hiding this comment

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

Should COMMITS_TREE_MERGED_PRERELEASES be renamed to COMMITS_TREE_AFTER_MERGED_PRERELEASES?

How is this test different than just test_generate_tree_from_commits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that name is better. It's different in the sense, that COMMITS_TREE_AFTER_MERGED_PRERELEASES does not contain pre-releases (there were 2) and the changes of them are now part of the next non-prerelease.

@woile
Copy link
Member

woile commented Apr 16, 2023

How does this feature affect existing changelogs?
Will it update old pre-releases as well? could we add a test for it?

@woile
Copy link
Member

woile commented Apr 16, 2023

Can you point this PR to v3? thanks!

@MoritzBoehme
Copy link
Contributor Author

How does this feature affect existing changelogs? Will it update old pre-releases as well? could we add a test for it?

Existing changelogs will be changed when run with this flag. This is unless you also add the --incremental, set the changelog_incremental config option to true or use cz bump --changelog (as this implies incremental). I am adding a test right now to ensure this leaves prior pre-releases untouched this way.

Can you point this PR to v3? thanks!

Will do!

@MoritzBoehme MoritzBoehme changed the base branch from master to v3 April 16, 2023 15:06
@MoritzBoehme MoritzBoehme force-pushed the merge-prerelease branch 2 times, most recently from bde4bdc to 44ace73 Compare April 16, 2023 15:16
@MoritzBoehme
ci: update codecov to newest version
558af4b
@MoritzBoehme
test(scripts): run same file tests individually
e34de55 
Running tests concurrently can cause temporary files, such as the file
used to backup the commit message, to be deleted by one test but
accessed by another test later on.
@MoritzBoehme MoritzBoehme requested a review from woile April 16, 2023 15:29
@woile woile merged commit 7a8c2d8 into commitizen-tools:v3 Apr 17, 2023
@Apkawa Apkawa mentioned this pull request Apr 17, 2023
Merged
4 tasks
Apkawa pushed a commit to Apkawa/commitizen that referenced this pull request Apr 17, 2023
@actions-user
test: fix test_changelog_incremental_with_merge_prerelease issue with…
3c9c89c 
… freeze_time

commitizen-tools#703
Apkawa pushed a commit to Apkawa/commitizen that referenced this pull request Apr 18, 2023
@actions-user
test(changelog): fix test_changelog_incremental_with_merge_prerelease…
77c19c4 
… issue with freeze_time

Signed-off-by: apkawa <[email protected]>

commitizen-tools#703
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woile woile Awaiting requested review from woile

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

codecov dev dependency is deprecated
2 participants
@MoritzBoehme @woile