-
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
cubicinterpolation: modernize for conan 2.0 #16299
cubicinterpolation: modernize for conan 2.0 #16299
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.
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.
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.
Ok, at least the pipeline is working now. |
This comment has been minimized.
This comment has been minimized.
What are the next steps for me? 🙂 |
This comment has been minimized.
This comment has been minimized.
Wait for review! huge backlog of PRs with 2.0 so you'll need some patients... a month is kinda normal now https://github.com/conan-io/conan-center-index/blob/master/docs/review_process.md for the rest of the details :) |
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.
Overall this looks really good, just a handful of small comments to help update this even more!
if self.settings.compiler.get_safe("cppstd"): | ||
check_min_cppstd(self, "14") | ||
|
||
if str(self.settings.compiler) == "Visual Studio" and Version(self.settings.compiler.version) < "16": |
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.
is raise = False would be better
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.
check_min_vs
with raise_invalid=False
has only been available since 1.57.0
(see #12880).
Should I just replace the block with a simple check_min_vs(self, 192)
? Otherwise I would have to change the required_conan_version to >=1.57.0
.
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.
ConanCenter recipes are always the latest (minus a version) Consumers are expected to be on 1.59 (sooner the later it will become 2.0) so it's absolutely allowed to increase the min version :)
recipes/cubicinterpolation/all/test_v1_package/test_package.cpp
Outdated
Show resolved
Hide resolved
Oh, I didn't know that the backlog was this immense, but it's understandable since there are over a thousand of packages... Thanks to everyone helping with this effort! 🙂 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I made prince-chrismc/conan-center-index-pending-review#1, there are some fun graphs at the bottom we are down from about 150 PRs in review to just over 60 👏 working are way through |
sha256: "3151d99ecbbddd4e57605ff1919bdf234d08336b47d369b9dc562acff780aaf7" | ||
patches: | ||
"0.1.4": | ||
- patch_file: "patches/patch_conanbuildinfo.txt" | ||
base_path: "source_subfolder" | ||
patch_type: "conan" | ||
- patch_file: "patches/patch_conan_basic_setup.txt" |
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.
Would you mind renaming this (and the old patch) to a .diff
this way github will apply formating and it will be easier to read 🙏
patches/rm_conan_basic_setup.diff
would be amazing 🤩
This comment has been minimized.
This comment has been minimized.
Amazing work! |
Conan v1 pipeline ✔️All green in build 19 (
Conan v2 pipeline (informative, not required for merge) ✔️
All green in build 18 ( |
* compatability of cubic_interpolation with Conan 2.0 * missing newline * use patch from new resource * invalid use of patch * adapt structure for patch * add patch for older version of package * safe delete of fPIC * disable VS<16 * fix VS check * add suggestions by @prince-chrismc * update organization name * clarify TODO in comment * rename patch files and remove duplicate cpp file * Apply suggestions from code review --------- Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Specify library name and version: cubicinterpolation/0.1.x
Hello. With this PR, I've tried to update the conan recipe of cubicinterpolation to be compatible with Conan 2.0
For now, I've updated in such a way that it maintains compatibility with Conan 1.x, I hope this is accepted.
I've oriented myself on the cctag package (#13402) since this recipe has a similar structure.
I've updated it in such a way that it works for me with Conan 1.54.0 and Conan 2.0.
I'm happy about feedback, so I can continue working on modernizing my other package (
proposal
) soon :)