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.

Sign up for GitHub

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

Jump to bottom

semver: improve constraint parsing #327

Merged
merged 1 commit into from
May 2, 2022
Merged

semver: improve constraint parsing #327

merged 1 commit into from
May 2, 2022

Conversation

abn
Copy link
Member

@abn abn commented Apr 19, 2022 •
edited
Loading

This change replaces the custom regex used with packaging.version.VERSION_PATTERN for consistency with other parts of the code base. Additionally, this fixes previous issues with parsing dev releases etc.

@abn abn requested a review from a team April 19, 2022 13:15
@radoering
Copy link
Member

Shouldn't the regex be fixed? This seems like doing workarounds.

@abn
Copy link
Member Author

abn commented Apr 27, 2022

Probably yes. But that requires a bit more testing I reckon as I am unsure what the impact of that is going to be else where and in poetry.

Will take a look.

@abn abn force-pushed the fix-dependency-constraint branch from c0e7a91 to 376dc7f Compare April 30, 2022 12:50
@abn abn changed the title constraints: handle pre-release dev tags constraints: improve and simplify parsing Apr 30, 2022
@abn abn force-pushed the fix-dependency-constraint branch from 376dc7f to ffb2dd1 Compare April 30, 2022 12:56
@abn abn changed the title constraints: improve and simplify parsing semver: improve constraint parsing Apr 30, 2022
@abn abn force-pushed the fix-dependency-constraint branch from ffb2dd1 to c920fc8 Compare April 30, 2022 12:57
@abn abn requested a review from radoering April 30, 2022 12:58
@abn
Copy link
Member Author

abn commented Apr 30, 2022

@radoering ended up doing a more comprehensive fix. Thanks for the nudge :)

radoering
radoering requested changes Apr 30, 2022
View reviewed changes
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

Much better than before. 👍 Just some minor nitpicks.

src/poetry/core/semver/helpers.py Outdated Show resolved Hide resolved
src/poetry/core/semver/patterns.py Outdated Show resolved Hide resolved
src/poetry/core/semver/patterns.py Outdated Show resolved Hide resolved
@abn
Copy link
Member Author

abn commented Apr 30, 2022

Let me know if you feel strongly about any of the comments I responded to.

@radoering
Copy link
Member

At the moment, we allow trailing white space for 2 out of 5 constraint patterns. If we allow it, shouldn't we do for all?

I only feel strongly against "Not common enough to be tested.". A test seems quite easy and makes clear that this part of the regex is not a mistake.

@abn
Copy link
Member Author

abn commented Apr 30, 2022

I agree. Will allow it in all of them as it makes sense. Tests too ;)

@abn abn force-pushed the fix-dependency-constraint branch from 7497cfc to 2fa9e3f Compare May 2, 2022 14:22
@abn
semver: improve constraint parsing
af7f815 
This change replaces the custom regex used with
`packaging.version.VERSION_PATTERN` for consistency with other parts of
the code base. Additionally, this fixes previous issues with parsing
pre-release dev releases etc.
@abn abn force-pushed the fix-dependency-constraint branch from 2fa9e3f to af7f815 Compare May 2, 2022 16:46
@abn abn requested a review from radoering May 2, 2022 16:46
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
1.5% 1.5% Duplication

@radoering radoering merged commit c5c5ca4 into python-poetry:master May 2, 2022
@abn abn deleted the fix-dependency-constraint branch May 2, 2022 18:13
@radoering radoering mentioned this pull request May 12, 2022
Closed
3 tasks
@finswimmer finswimmer mentioned this pull request May 20, 2022
Merged
bostonrwalker pushed a commit to bostonrwalker/poetry-core that referenced this pull request Aug 29, 2022
@abn @bostonrwalker
semver: improve constraint parsing (python-poetry#327)
a6fb2a7 
This change replaces the custom regex used with
`packaging.version.VERSION_PATTERN` for consistency with other parts of
the code base. Additionally, this fixes previous issues with parsing
pre-release dev releases etc.
DavidVujic pushed a commit to DavidVujic/poetry-core that referenced this pull request Aug 31, 2022
@abn @DavidVujic
semver: improve constraint parsing (python-poetry#327)
ca9016e 
This change replaces the custom regex used with
`packaging.version.VERSION_PATTERN` for consistency with other parts of
the code base. Additionally, this fixes previous issues with parsing
pre-release dev releases etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radoering radoering radoering approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@abn @radoering