-
Notifications
You must be signed in to change notification settings - Fork 96
Feature/769 libuv package and thread header #815
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #815 +/- ##
==========================================
+ Coverage 91.43% 91.50% +0.06%
==========================================
Files 234 235 +1
Lines 28655 28686 +31
==========================================
+ Hits 26200 26248 +48
+ Misses 2455 2438 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I will finish reviewing this weekend. |
libs/utils/CMakeLists.txt
Outdated
| find_package(libuv REQUIRED) | ||
|
|
||
| if (NOT TARGET libuv::uv AND TARGET uv) | ||
| #Note: conan libuv package 1.49.2 defines uv target, but 1.51.0 defines libuv::uv target |
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.
Using latest conan 1.x is OK, thus this hack can be removed.
Let update our conan 1.x all to the latest.
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.
Running conan1 gives the following notice:
WARN: **************************************************
WARN: *** Conan 1 is legacy and on a deprecation path **
WARN: *********** Please upgrade to Conan 2 ************
WARN: **************************************************
I think it's time to get rid of Conan 1.x.
PengZheng
left a comment
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.
Nice addition! LGTM
I left some minor remarks.
This PR adds the libuv dependency and creates initial celix_uv_cleanup.h and tests.
Follow up PRs will replace celix_threads.h usage with libuv variants.