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

Implement SemanticVersionRange to handle ranges #181

Closed
wants to merge 2 commits into from

Conversation

CasperWA
Copy link
Collaborator

It builds on the pip Requirement and Specifier classes to deal with version ranges, determining whether a given version is part of it and handling how a set of version specifiers should be updated according to the latest detected version.

Fixes #141

There's a minor regression, in the sense that non-contiguous version ranges are no longer supported without using != to specify the non-inclusive versions.
This means a version range like <2,>=3 is not allowed, instead one should do: !=2.*.
The change is due to the way the version ranges is implemented - only a single upper and lower bound is allowed per version specifier set. For the disallowed specifier set in the example above there are indeed two distinct ranges: One from v0 to immediately prior to v2, and then another from v3 to infinity. Whereas for the allowed specifier set (which results in the same actual version range) there is only a single lower and upper bound: v0 and infinity, respectively.

In order to be ready for review the following is needed:

  • Possibly more tests for the new class.
  • Check if the "ignore version rules"-implementation can be rewritten using the new class as well as pip Requirement/Specifier/...
  • Update the documentation, specifically with a page about the version update strategy currently "hard-coded" into the logic.

It builds on the pip Requirement and Specifier classes to deal with
version ranges, determining whether a given version is part of it and
handling how a set of version specifiers should be updated according to
the latest detected version.
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #181 (487e837) into main (c030b97) will decrease coverage by 3.85%.
The diff coverage is 62.46%.

@@            Coverage Diff             @@
##             main     #181      +/-   ##
==========================================
- Coverage   71.42%   67.58%   -3.85%     
==========================================
  Files           9        9              
  Lines         567      873     +306     
==========================================
+ Hits          405      590     +185     
- Misses        162      283     +121     
Files Changed Coverage Δ
ci_cd/utils.py 73.21% <61.96%> (-21.27%) ⬇️
ci_cd/tasks/update_deps.py 74.64% <63.41%> (-6.68%) ⬇️

@CasperWA
Copy link
Collaborator Author

CasperWA commented Sep 29, 2023

Hmm, I'm doubting myself on this implementation. It seems to me there is an "easier" solution here:

Create some "ignore" rules based on the versions listed, add any other relevant "ignore" rules and then let that whole existing logic take care of it.

That would remove the need for the new "range" class entirely, really. I think.
Another solution would be to go the other way around and let the new "range" class handle the ignore rules as well, but I think that's foolish - even though the logic handling it is currently not as "clean".


There are some good things implemented and used here though - like looking into the pip package for ways of reading requirements, versions, etc. That should be implemented and used here, I think. That just makes the most sense (for Python-based repositories anyway).

@CasperWA CasperWA mentioned this pull request Oct 9, 2023
2 tasks
@CasperWA
Copy link
Collaborator Author

Close in favor of #190

@CasperWA CasperWA closed this Nov 10, 2023
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.

Ensure version dependency ranges are respected when updating
1 participant