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(semver): improved support of the Python Version Identification part of PEP440 #3474

Merged
merged 7 commits into from
Mar 25, 2024

Conversation

icj217
Copy link
Contributor

@icj217 icj217 commented Mar 22, 2024

The semver util's current version logic does not fully implement/adhere to PEP440 in two respects:

  • There is no support for "local version identifiers", which are basically the same as SemVer's build metadata (e.g. 1.2.3+foobar), when used in conjunction with a pre-release label.
  • The current pre-release logic doesn't reflect the ability for python pre-releases to include post-release and developmental release labels in conjunction with the pre-release itself (e.g. 1.2.3.rc1.post2.dev3)

This PR addresses these gaps so that the python release supports these features. I've kept support for the pre label as a synonym for dev.

Projen and JSII's logic diverged ever so slightly re: including a . between the version and pre-release labels (e.g. projen was yielding 1.2.3rc0 while JSII would yield 1.2.3.rc0. I decided to update projen's logic to mirror that from JSII.

E.g. now 1.2.3-rc.1.dev.2.post.3+foobar will now yield 1.2.3.rc1.post3.dev2+foobar for python packages.

I have an identical PR open over in jsii: aws/jsii#4462


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link

codecov-commenter commented Mar 25, 2024

Codecov Report

Attention: Patch coverage is 80.59701% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 96.33%. Comparing base (41a2b73) to head (44559db).
Report is 7 commits behind head on main.

Files Patch % Lines
src/util/semver.ts 80.59% 13 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3474   +/-   ##
=======================================
  Coverage   96.32%   96.33%           
=======================================
  Files         191      191           
  Lines       37407    37445   +38     
  Branches     3489     3498    +9     
=======================================
+ Hits        36032    36071   +39     
+ Misses       1375     1374    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@icj217 icj217 marked this pull request as ready for review March 25, 2024 13:54
@mrgrain mrgrain changed the title fix(semver): Improve python release version conversion logic fix(semver): improved support of the Python Version Identification part of PEP440 Mar 25, 2024
@mrgrain
Copy link
Contributor

mrgrain commented Mar 25, 2024

@icj217 Thanks for this! I suspect this needs a new release of pacmak with the respective change first?

Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

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

See question

@icj217
Copy link
Contributor Author

icj217 commented Mar 25, 2024

@icj217 Thanks for this! I suspect this needs a new release of pacmak with the respective change first?

@mrgrain Not necessarily. I think this can be merged first because this logic applies to any python package (not just a JSII-based one) that a user registers as a dependency in their project. JSII projects won't be able to publish versions that would exercise the new logic here until the pacmak release is published.

@mergify mergify bot merged commit 2ec610d into projen:main Mar 25, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants