Skip to content

chore: prevent normalization of semver versioning #1292

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

Merged
merged 3 commits into from
Apr 16, 2021

Conversation

dandhlee
Copy link
Contributor

When there is a patch version added to semver versioning, setuptools.setup(version) will normalize the versioning from -patch to .patch which is not correct SEMVER versioning. The added feature with setuptools.sic(version) will prevent this from happening.

@dandhlee dandhlee requested a review from a team as a code owner April 14, 2021 23:29
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 14, 2021
@parthea parthea added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 15, 2021
@parthea
Copy link
Contributor

parthea commented Apr 15, 2021

This PR is on hold as @dandhlee confirmed that this requires a minimum version of setuptools.

@dandhlee dandhlee removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 15, 2021
@dandhlee
Copy link
Contributor Author

All is good now, if there is no problem with Kokoro pre-submits then it's fine, otherwise I'm working through the repositories to make necessary change and workarounds.

@busunkim96
Copy link
Contributor

@dandhlee This repo doesn't declare setuptools as a direct dependency, so please use the workaround to avoid breaking folks.

You won't see a failure in CI since the tests will install the latest version of setuptools in a fresh environment.

@dandhlee
Copy link
Contributor Author

Understood, will make those changes around! Sorry for that 😓

@dandhlee dandhlee added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 15, 2021
@dandhlee dandhlee removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 16, 2021
Copy link
Contributor

@parthea parthea left a comment

Choose a reason for hiding this comment

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

LGTM

@parthea parthea added the automerge Merge the pull request once unit tests and other checks pass. label Apr 16, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit 937ae84 into master Apr 16, 2021
@gcf-merge-on-green gcf-merge-on-green bot deleted the remove-semver-normalization branch April 16, 2021 20:30
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Apr 16, 2021
parthea added a commit that referenced this pull request Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants