Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 304
Add version_type option to make version compatibly with semver#686
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Apkawa commented Mar 13, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
codecovbot commented Mar 13, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@## v3 #686 +/- ## ========================================== - Coverage 98.11% 97.92% -0.19% ========================================== Files 41 42 +1 Lines 1855 1928 +73 ========================================== + Hits 1820 1888 +68 - Misses 35 40 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
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. |
woile commented Mar 13, 2023
Hey this is amazing! Thanks for taking the time to do it. We have to think of a different name instead of |
Apkawa commented Mar 13, 2023
hmm, maybe |
woile commented Mar 13, 2023
I was thinking of |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
woile commented Mar 13, 2023
Please also add the setting to config.md |
woile commented Mar 13, 2023
@Lee-W wanna take a look? I left some minor comments, but in general it looks good. Do you have any thoughts or thing we should we aware of? I'm thinking, once we merge this, we should rebase |
woile commented Mar 13, 2023
You can run |
Apkawa commented Mar 13, 2023
Oh, i checked only 3.9. Hmm |
Apkawa commented Mar 13, 2023
Please restart checks |
Apkawa commented Mar 13, 2023
Lee-W commented Mar 13, 2023
I'm on a trip these 2 weeks and probably won't be able to review it. |
version_provider option to make version compatibly with semverversion_type option to make version compatibly with semverAliSajid commented Apr 9, 2023
Bumping this since this is really important for me. |
Lee-W left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Apkawa This feature is amazing! Thanks for sending this awesome PR. I think we're almost ready to merge this one. I left a few minor comments. Let me know your thought.
commitizen/commands/changelog.py Outdated
| ) | ||
| version_type=self.config.settings.get("version_type") | ||
| self.version_type=version_typeandversion_types.types[version_type] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we capitalize types? i.e. version_types.TYPES[version_type]. Or maybe VERSION_TYPES might be an even better name?
It took me some time to find out version_type and version_type are different
commitizen/version_types.py Outdated
| classVersionProtocol(_Protocol): | ||
| def__init__(self, _version: typing.Union[Version, str]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: I prefer from typing import Union, Optional a bit more
commitizen/version_types.py Outdated
| parts.append(".".join(str(x) forxinversion.release)) | ||
| # Pre-release | ||
| ifversion.preisnotNone: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: I prefer if not version.pre a bit more
docs/config.md Outdated
| |`use_shortcuts`|`bool`|`false`| If enabled, commitizen will show keyboard shortcuts when selecting from a list. Define a `key` for each of your choices to set the key. [See more][shortcuts]| | ||
| |`major_version_zero`|`bool`|`false`| When true, breaking changes on a `0.x` will remain as a `0.x` version. On `false`, a breaking change will bump a `0.x` version to `1.0`. [major-version-zero]| | ||
| |`prerelease_offset`|`int`|`0`| In special cases it may be necessary that a prerelease cannot start with a 0, e.g. in an embedded project the individual characters are encoded in bytes. This can be done by specifying an offset from which to start counting. [prerelease-offset]| | ||
| |`version_type`|`str`|`pep440`| Select a version type from the following options [`pep440`, `semver`]. Useful for non python projects. [See more][version_type]| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: could you please change non python to non-python? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
tests/commands/test_bump_command.py Outdated
| tag_exists=git.tag_exist("0.2.0-a0") | ||
| asserttag_existsisTrue | ||
| withopen(tmp_version_file, "r") asf: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: do you think we could try the following?
forversion_filein (tmp_version_file, tmp_commitizen_cfg_file): withopen(version_file, "r") asf: assert"0.2.0-a0"inf.read()There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you agree with this, maybe we could try this technique in other tests in this PR as well
tests/commands/test_bump_command.py Outdated
| # PRERELEASE | ||
| tmp_version_file=tmp_commitizen_project.join("__version__.py") | ||
| tmp_version_file.write("0.1.0") | ||
| tmp_commitizen_cfg_file=tmp_commitizen_project.join("pyproject.toml") | ||
| tmp_version_file_string=str(tmp_version_file).replace("\\", "/") | ||
| tmp_commitizen_cfg_file.write( | ||
| f"{tmp_commitizen_cfg_file.read()}\n" | ||
| f'version_files = ["{tmp_version_file_string}"]\n' | ||
| f'version_type = "semver"' | ||
| ) | ||
| create_file_and_commit("feat: new user interface") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These setups are similar across different tests. Would it be possible for us to make them pytest fixtures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I wrote a simple fixture returning a function with parameters ea7ac93#diff-e52e4ddd58b7ef887ab03c04116e676f6280b824ab7469d5d3080e5cba4f2128R30
| out, _=capsys.readouterr() | ||
| assertout=="## 0.3.0-a1 (2022-02-13)\n\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we unify the test way to use pytest-regressions as well?
| @pytest.mark.parametrize( | ||
| "test_input,expected", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: please add an additional space i.e., test_input, expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Apkawa commented Apr 16, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
hmm. CI failed. but i didn't touch dependencies. Do I need to merge the master onto a branch? |
Lee-W commented Apr 17, 2023
I just found out the root cause. It seems to relate to https://about.codecov.io/blog/message-regarding-the-pypi-package/. Could you please remove |
Lee-W commented Apr 17, 2023
Other than that, everything looks good to me |
woile commented Apr 17, 2023
Could you point this to I'll be working on v3 this week, and I would like this to be part of it 💪🏻 |
Apkawa commented Apr 17, 2023
Ok. I fixed CI. |
woile commented Apr 17, 2023
Could you point the PR to the branch |
Apkawa commented Apr 17, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Done. Failed not my test. In #703test_changelog_incremental_with_merge_prerelease need add i may fix it. |
woile commented Apr 17, 2023
Thanks, if you can also rebase your pr on top of V3 so we avoid the merge commit. I can fix the tests myself this week |
Apkawa commented Apr 17, 2023
Ok, i can do a rebase to one commit, but I'm afraid that if I do a force push, this PR will close. |
woile commented Apr 18, 2023
I'm asking for a rebase, not a squash. Otherwise the timeline will be dirty. and then follow the instructions, fix the conflict and continue the rebase until it's done After that, you can do a force push and the PR will remain open |
…ompatible with semver Signed-off-by: apkawa <[email protected]>
… rename to `pep440` Signed-off-by: apkawa <[email protected]>
Signed-off-by: apkawa <[email protected]>
…rsion subclass Signed-off-by: apkawa <[email protected]>
Signed-off-by: apkawa <[email protected]>
…ersion_type=semver` option Signed-off-by: apkawa <[email protected]>
… issue with freeze_time Signed-off-by: apkawa <[email protected]>
Apkawa commented Apr 18, 2023
done, please check |
woile commented Apr 19, 2023
Thank you so much! 🎉 |
* feat(bump): version_provider=semver optional option to make version compatible with semver Signed-off-by: apkawa <[email protected]> * refactor(bump): version_provider rename to version_type; `pep` option rename to `pep440` Signed-off-by: apkawa <[email protected]> * docs(bump): add `version_type` info to config.md Signed-off-by: apkawa <[email protected]> * refactor(bump): to use VersionProtocol interface instead packaging.Version subclass Signed-off-by: apkawa <[email protected]> * test(bump): `VersionProtocol` workaround mypy for py==3.7 Signed-off-by: apkawa <[email protected]> * fix(changelog): `changelog` command does not find version tag with `version_type=semver` option Signed-off-by: apkawa <[email protected]> * refactor: minor review fixes * test(changelog): fix test_changelog_incremental_with_merge_prerelease issue with freeze_time Signed-off-by: apkawa <[email protected]> --------- Signed-off-by: apkawa <[email protected]>

Description
I decided to use it in a rust project but cargo crashed when trying to use the prerelease versions.
I have added an optional setting that solves my problem for now.
version_typeChoose version type
pep440- default version provider.1.0.1a01.0.1dev01.0.1a0.dev0semver- semver compatibly provider. Added "-" delimiter1.0.1-a01.0.1-dev01.0.1-a0-dev0TODO: renamed pre to long name, like alpha, beta, ..etc via additional option
semver_longChecklist
./scripts/formatand./scripts/testlocally to ensure this change passes linter check and testExpected behavior
Steps to Test This Pull Request
Additional context
#150