-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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(cargo): handle version range using Less Than Equal #9828
fix(cargo): handle version range using Less Than Equal #9828
Conversation
@@ -141,7 +141,11 @@ def update_range_requirements(string_reqs) | |||
raise UnfixableRequirement if req.start_with?(">") | |||
|
|||
req.sub(VERSION_REGEX) do |old_version| | |||
update_greatest_version(old_version, target_version) | |||
if req.start_with?("<=") | |||
version_class.new(target_version) |
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.
version_class.new(target_version) | |
target_version |
This branch is returning a Cargo::Version
instance, which will be converted to a string with #to_s
, which just outputs the version string it was initialized with, so I think this could be simplified.
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.
That makes sense. Fixed.
@@ -151,17 +151,34 @@ | |||
end | |||
|
|||
context "when there were multiple range specifications" do | |||
let(:req_string) { "> 1.0.0, < 1.2.0" } | |||
its([:requirement]) { is_expected.to eq("> 1.0.0, < 1.6.0") } | |||
context "with `less then`" do |
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.
Non-blocking/nitpick: technically less than
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.
Fixed
context "when including a pre-release" do | ||
let(:req_string) { ">=1.2.0, <1.4.0-dev" } | ||
its([:requirement]) { is_expected.to eq(">=1.2.0, <1.6.0") } | ||
context "with `less then equal`" do |
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.
Non-blocking/nitpick: technically less than or equal to
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.
Fixed
When a version range with a '<' character is found, the range is updated to the (latest version + 1). However for a range using "<=" this is invalid. Add a special case for a range using Less Than Equal which is updated to simply the latest version.
2f03293
to
72f1102
Compare
I fixed all comments and resolved the merge conflict. |
When a version range with a '<' character is found, the range is updated to the (latest version + 1). However for a range using "<=" this is invalid. Add a special case for a range using Less Than Equal which is updated to simply the latest version.