-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
aws-c-sdkutils: add v0.1.13, use version range for dependencies #22406
aws-c-sdkutils: add v0.1.13, use version range for dependencies #22406
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
LGTM, except for the version range use. It is only allowed for a small set of packages on CCI currently, like libcurl
and zlib
.
Hi @valgur , Thanks for your review. Could you guide me any suggestion to use version range here ? |
self.requires("aws-c-common/0.8.2", transitive_headers=True, transitive_libs=True) | ||
elif Version(self.version) == "0.1.12": | ||
self.requires("aws-c-common/[>=0.9.4 <=0.9.10]", transitive_headers=True, transitive_libs=True) |
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.
self.requires("aws-c-common/[>=0.9.4 <=0.9.10]", transitive_headers=True, transitive_libs=True) | |
# [>=0.9.4 <=0.9.10] | |
self.requires("aws-c-common/0.9.10", transitive_headers=True, transitive_libs=True) |
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.
Thanks for pointing this out. I made the change accordingly.
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.
There is a minor difference. Because the current recipe of was-c-common
doesn't have 0.9.10
, I use 0.9.6
here.
else: | ||
self.requires("aws-c-common/0.9.6", transitive_headers=True, transitive_libs=True) | ||
self.requires("aws-c-common/[>=0.9.10]", transitive_headers=True, transitive_libs=True) |
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.
self.requires("aws-c-common/[>=0.9.10]", transitive_headers=True, transitive_libs=True) | |
# [>=0.9.10] | |
self.requires("aws-c-common/0.9.12", transitive_headers=True, transitive_libs=True) |
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.
Thanks for pointing this out. I made the change accordingly.
7c3b549
to
f527702
Compare
f527702
to
c1a874e
Compare
Conan v1 pipeline ✔️All green in build 2 (
Conan v2 pipeline ✔️
All green in build 2 (
|
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.
LGTM but in a future it will be good to handle version mappings from conandata.yml to avoid regeneration of new revisions
Specify library name and version: aws-c-sdkutils/0.1.13
Update this dependency because I want to update
aws-crt-cpp
to the latest version. Here are related issues:The version range is based on the submodules used in
aws-crt-cpp
. I wrote a script to collect the version mapping betweenaws-c-sdkutils
andaws-c-common
. In the following mapping, keys are the versions ofaws-c-sdkutils
and values are the versions ofwas-c-common
.The change of version range in this PR is based on it.