Feature/769 libuv package and thread header #815
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.53% +0.10%
==========================================
Files 234 235 +1
Lines 28655 28687 +32
==========================================
+ Hits 26200 26260 +60
+ Misses 2455 2427 -28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I will finish reviewing this weekend. |
| extern "C" { | ||
| #endif | ||
|
|
||
| CELIX_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(uv_thread_t, uv_thread_join) |
There was a problem hiding this comment.
I'm not sure whether this one makes sense, since it can only be used with joinable thread and detached threads are used a lot in my day job.
| 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.
Using latest conan 1.x is OK, thus this hack can be removed.
Let update our conan 1.x all to the latest.
| # LIBUV_INCLUDE_DIR - The libuv include directory | ||
| # LIBUV_LIBRARY - The libuv library | ||
| # uv - Imported target for libuv | ||
|
|
There was a problem hiding this comment.
cmake/CelixDeps.cmake.in should also be updated.
| find_package_handle_standard_args(libuv DEFAULT_MSG LIBUV_LIBRARY LIBUV_INCLUDE_DIR) | ||
| mark_as_advanced(LIBUV_INCLUDE_DIR LIBUV_LIBRARY) | ||
| endif () | ||
|
|
PengZheng
left a comment
PengZheng
left a comment
There was a problem hiding this comment.
Nice addition! LGTM
I left some minor remarks.
3 participants
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.