Feature: version schemes endpoint #733

noirbizarre · 2023-05-01T15:29:24Z

Description

This PR is a continuation of #686 following this comment.

It does the following:

rename version_type into scheme to follow specifications semantic (PEP 440, SemVer but also CalVer which are all using the scheme term)
expose a commitizen.scheme endpoint for version scheme plugins
move some methods from bump to the plugin allowing support for all commitizen feature (especially incremental versioning which is relying on the ability to parse version as well as bumping which should be able to generate the next version according to the scheme)

This PR is also related to #645 as the incremental feature requires both the template and the version knowledge to be complete (aka. one of the 2 PR should be rebased after the merge of the other to properly handle this part)

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

codecov · 2023-05-01T15:34:03Z

Codecov Report

Patch coverage: 98.75% and project coverage change: +0.16 🎉

Comparison is base (eb39f8b) 97.31% compared to head (49572ab) 97.47%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #733      +/-   ##
==========================================
+ Coverage   97.31%   97.47%   +0.16%     
==========================================
  Files          42       42              
  Lines        2045     2061      +16     
==========================================
+ Hits         1990     2009      +19     
+ Misses         55       52       -3

Flag	Coverage Δ
unittests	`97.47% <98.75%> (+0.16%)`	⬆️

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

Impacted Files	Coverage Δ
...en/cz/conventional_commits/conventional_commits.py	`100.00% <ø> (ø)`
commitizen/providers.py	`97.27% <75.00%> (-0.03%)`	⬇️
commitizen/version_schemes.py	`98.42% <98.42%> (ø)`
commitizen/__version__.py	`100.00% <100.00%> (ø)`
commitizen/bump.py	`100.00% <100.00%> (ø)`
commitizen/changelog.py	`100.00% <100.00%> (ø)`
commitizen/changelog_parser.py	`96.96% <100.00%> (+0.04%)`	⬆️
commitizen/cli.py	`94.20% <100.00%> (ø)`
commitizen/commands/bump.py	`98.17% <100.00%> (+0.02%)`	⬆️
commitizen/commands/changelog.py	`98.94% <100.00%> (ø)`
... and 14 more

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

commitizen/schemes.py

commitizen/cli.py

woile · 2023-05-02T17:30:49Z

This looks really cool. I really like using the proper name: "scheme".

My main concern is that this a big breaking change, right? I already migrated a lot of projects to version_type at work and recently released a lot of v3 related things for commitizen. I would rather do it in steps to give me some buffer:

Keep in the config version_type and --version-type with a deprecation warning:

version-type will be deprecated in the next major release, please use "version-scheme" instead.

On v4 we remove version-type

I also think using version-scheme in file names and as config and cli args, is more clear than "scheme". I don't think using just "scheme" gives enough context to users.

noirbizarre · 2023-05-07T17:36:09Z

OK, I rebased and squash the PR, taking every remark into account (this includes the deprecation warnings).

Note: I wonder if exposing this as a cli parameter (--version-scheme, was --version-type before) makes sense: I can't see a use case where you have commitizen properly configured for your project and you need to provide another version scheme on the command line.

noirbizarre · 2023-05-13T20:42:55Z

@Lee-W @woile this one is ready as well as #645.
Whatever order you choose to merge, the second one will need a rebase to resolve conflict and integrate the other.

woile · 2023-05-20T17:33:26Z

Looks really good, sorry for the delay. I will take a deeper look when i'm back from holidays (June). Thanks a lot 🚀

Lee-W · 2023-06-03T04:36:24Z

This is fantastic! I am interested in delving into it further in the upcoming weeks. I have been extremely busy this year and feeling a bit overwhelmed but will try my best to help!

woile · 2023-06-03T08:18:32Z

commitizen/version_schemes.py

+DEFAULT_VERSION_PARSER = r"v?(?P<version>([0-9]+)\.([0-9]+)\.([0-9]+)(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?(?:\+[0-9A-Za-z-]+)?(\w+)?)"
+
+
+class VersionProtocol(Protocol):


I was thinking the VersionProvider(ABC) is an ABC instead of Protocol, should we stick to ABC for consistency? Or change VersionProvider to a Protocol

btw how does ABC work with type inference? I'd like to have something that can type hint (this helps during development if you use mypy or similar), and that can also fail at runtime if not implemented 🤔 (this for those who don't use any kind of tpye hinting)

We need a compromise here:

ABC forces you into inheritance, which is defeating protocol purpose (especially Feature: version schemes endpoint #733 (comment)) and it would force Pep440 and SemVer into multiple inheritance

Protocol is the opposite, it forces into nothing because by default it is ignored at runtime

However, there is one possibility which is interesting: we can make the protocol runtime checkable and raise a warning or fail when calling get_version_scheme, something like:

scheme = ep.load() if not isinstance(scheme, VersionProtocol) warnings.warn(f"{name} is not implementing the Version protocol") return scheme

This way we would have best of both world:

clean type hinting

freedom of implementation

runtime error or warning for devs not using MyPy

What do you thing of this solution ?

I think it's great. I would also like to choose one between protocol and ABC. Do we need then to keep versionProvider as an ABC? or can we migrate (at some point in the future) to Protocol with runtime check?

OK, it has been added in the latest push (with some tests)

I think there is nothing preventing from switching between ABC and Protocol. In both cases, this is a design choice, the same as above 👆🏼

Totally, I just think we should stick to one for plugins, to reduce cognitive load on those who will implement them.

woile · 2023-06-03T08:25:37Z

This looks awesome, I think it's ready to be merged (besides my comment). Thanks @noirbizarre

…me` endpoint. Refactors `version_types` (which is now deprecated) into `version_schemes`, exposes them through a `commitizen.scheme` entrypoint and ensure it encapsulates all the versionning logic for parsing and bumping.

noirbizarre · 2023-06-06T23:51:17Z

PR updated with the protocol runtime check and the associated tests 👍🏼
It should be ready to merge 🚀 🎉

woile · 2023-06-20T06:33:43Z

Sorry for the delay, I'm a bit slow, this looks good to me, shall we merge?

Lee-W · 2023-06-20T08:07:26Z

@woile Yep, let's merge it 💪 I think I'll have some time to take a look this holiday. I'll point out and maybe try to address if I found anything. Thanks!

noirbizarre force-pushed the feature/schemes branch 2 times, most recently from a65da14 to b4b9716 Compare May 1, 2023 17:47

woile reviewed May 2, 2023

View reviewed changes

commitizen/schemes.py Outdated Show resolved Hide resolved

woile reviewed May 2, 2023

View reviewed changes

commitizen/cli.py Outdated Show resolved Hide resolved

noirbizarre force-pushed the feature/schemes branch from b4b9716 to 55f470a Compare May 7, 2023 17:33

noirbizarre force-pushed the feature/schemes branch 4 times, most recently from 396dcdd to ac5f873 Compare May 13, 2023 19:32

woile reviewed Jun 3, 2023

View reviewed changes

noirbizarre added 2 commits June 7, 2023 01:39

style(mypy): ignore py.* legacy pytest imports

49572ab

noirbizarre force-pushed the feature/schemes branch from ac5f873 to 49572ab Compare June 6, 2023 23:47

woile requested a review from Lee-W June 20, 2023 06:33

woile approved these changes Jun 20, 2023

View reviewed changes

woile merged commit 0f5e8bf into commitizen-tools:master Jun 20, 2023

noirbizarre deleted the feature/schemes branch June 20, 2023 08:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: version schemes endpoint #733

Feature: version schemes endpoint #733

noirbizarre commented May 1, 2023 •

edited

Loading

codecov bot commented May 1, 2023 •

edited

Loading

woile commented May 2, 2023

noirbizarre commented May 7, 2023

noirbizarre commented May 13, 2023

woile commented May 20, 2023

Lee-W commented Jun 3, 2023

woile Jun 3, 2023 •

edited

Loading

woile Jun 3, 2023

noirbizarre Jun 6, 2023 •

edited

Loading

woile Jun 6, 2023

noirbizarre Jun 6, 2023

woile Jun 7, 2023

woile commented Jun 3, 2023

noirbizarre commented Jun 6, 2023

woile commented Jun 20, 2023

Lee-W commented Jun 20, 2023

		DEFAULT_VERSION_PARSER = r"v?(?P<version>([0-9]+)\.([0-9]+)\.([0-9]+)(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?(?:\+[0-9A-Za-z-]+)?(\w+)?)"


		class VersionProtocol(Protocol):

Feature: version schemes endpoint #733

Feature: version schemes endpoint #733

Conversation

noirbizarre commented May 1, 2023 • edited Loading

Description

Checklist

codecov bot commented May 1, 2023 • edited Loading

Codecov Report

woile commented May 2, 2023

noirbizarre commented May 7, 2023

noirbizarre commented May 13, 2023

woile commented May 20, 2023

Lee-W commented Jun 3, 2023

woile Jun 3, 2023 • edited Loading

Choose a reason for hiding this comment

woile Jun 3, 2023

Choose a reason for hiding this comment

noirbizarre Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

woile Jun 6, 2023

Choose a reason for hiding this comment

noirbizarre Jun 6, 2023

Choose a reason for hiding this comment

woile Jun 7, 2023

Choose a reason for hiding this comment

woile commented Jun 3, 2023

noirbizarre commented Jun 6, 2023

woile commented Jun 20, 2023

Lee-W commented Jun 20, 2023

noirbizarre commented May 1, 2023 •

edited

Loading

codecov bot commented May 1, 2023 •

edited

Loading

woile Jun 3, 2023 •

edited

Loading

noirbizarre Jun 6, 2023 •

edited

Loading