-
Notifications
You must be signed in to change notification settings - Fork 116
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
Allow Clipper2 to be supplied externally when using Manifold as a submodule #1115
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1115 +/- ##
==========================================
+ Coverage 91.46% 91.54% +0.07%
==========================================
Files 30 30
Lines 5908 5904 -4
==========================================
+ Hits 5404 5405 +1
+ Misses 504 499 -5 ☔ View full report in Codecov by Sentry. |
What are you waiting for to make this not a Draft? |
My own manual testing across platforms, which takes some time. I mostly wanted to make sure this is conceptually ok with you, plus make sure Manifold's tests pass. |
This is good to go now! |
LGTM. I'm wondering if we should do something similar for other dependencies, e.g. tbb. |
Yeah, it could make sense to align the deps lookups. I didn't run into the same issue with tbb as we're always supplying it as a system dependency from the OpenSCAD side. |
Another idea would be to hide all the details in custom Find macros, but I didn't want to touch too much as it's so time consuming to test it.. |
We typically just write some simple test in our CI, like https://github.com/elalish/manifold/blob/master/scripts/test-cmake-subdir.sh It should not be too hard to test. I can have a look at it later, and probably deal with #1105. Our CI jobs will probably grow by 5... I'm starting to think if we are kind of abusing GitHub action :P |
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!
Clipper2_FOUND
before attempting to find Clipper2