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.

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

Fix SCM provider to only search tags that are reachable by the current commit #975

Merged
merged 3 commits into from
Feb 4, 2024

Conversation

chadrik
Copy link
Contributor

@chadrik chadrik commented Jan 30, 2024

Description

The scm provider currently uses all of the tags in the repo to determine the current version, but it should instead only consider tags that are upstream from the current commit. This means commitizen currently consider tags that are in the future or on other branches when determining the version. This doesn't make sense. The pep621 provider doesn't determine the current version from the latest commit of pyproject.toml or from commits on other branches, it looks at the current state of the repo.

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

The expected behavior is that the current version should be determined by the "highest" version tag reachable by the current commit. In order to determine "highest" I had to add sortability to VersionProtocol, but luckily this is already implemented by BaseVersion.

Steps to Test This Pull Request

Additional context

@chadrik chadrik force-pushed the scm-tagging-workflow branch from 4768ece to 3efca40 Compare February 3, 2024 01:52
Copy link

codecov bot commented Feb 3, 2024

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (120d514) 97.33% compared to head (c09e00c) 97.38%.
Report is 169 commits behind head on master.

Files Patch % Lines
commitizen/cli.py 82.14% 5 Missing ⚠️
commitizen/providers/scm_provider.py 92.68% 3 Missing ⚠️
commitizen/git.py 86.66% 2 Missing ⚠️
commitizen/changelog_formats/__init__.py 97.77% 1 Missing ⚠️
commitizen/version_schemes.py 97.43% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #975      +/-   ##
==========================================
+ Coverage   97.33%   97.38%   +0.04%     
==========================================
  Files          42       55      +13     
  Lines        2104     2373     +269     
==========================================
+ Hits         2048     2311     +263     
- Misses         56       62       +6     
Flag Coverage Δ
unittests 97.38% <97.95%> (+0.04%) ⬆️

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.

@chadrik chadrik force-pushed the scm-tagging-workflow branch 2 times, most recently from dca166e to f0dc835 Compare February 3, 2024 02:29
@chadrik
Copy link
Contributor Author

chadrik commented Feb 3, 2024

codecov is failing, but I don't know what else to test. Any ideas?

The scm provider requires tags to be the source of truth for the current version, instead of files.  Therefore, the current version should be the highest tag version upstream from the current commit.
@chadrik chadrik force-pushed the scm-tagging-workflow branch from f0dc835 to dfd0b0c Compare February 3, 2024 02:35
@chadrik chadrik force-pushed the scm-tagging-workflow branch from b9013b0 to 23516be Compare February 3, 2024 17:53
@chadrik
Copy link
Contributor Author

chadrik commented Feb 3, 2024

I keep adding tests but the codecov is not improving. Any suggeestions?

@woile woile merged commit c1b1386 into commitizen-tools:master Feb 4, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants