-
Notifications
You must be signed in to change notification settings - Fork 22
Test cmake install and build examples #578
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
Test cmake install and build examples #578
Conversation
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.
I don't think it is good idea to remove complex_io.cpp.
It would be much better to fix it by adding
#include <h5cpp/contrib/stl/stl.hpp>
to the file. See pr_578.
After #566 one needs to include the contrib TypeTrait definitions explicitly.
It would be good also to fix the TypeTrait<Image>.get() i.e.
--- a/examples/image_h5.hpp
+++ b/examples/image_h5.hpp
@@ -72,6 +72,7 @@ class TypeTrait<Image<PixelT>>
{
const static DataspaceType & cref_ = Simple(hdf5::Dimensions{value.ny(),value.nx()},
hdf5::Dimensions{value.ny(),value.nx()});
+ return cref_;
}
static void *ptr(Image<PixelT> &value)
|
Also added option (off by default as this was the existing behaviour) to build the examples in the main CMake file ( |
|
Incidentally, I think that's the codebase warning-free now so may be worth adding an option (say |
jkotan
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.
Looks good to me
It sounds good. But then probably the macos tests fail because |
Yes, that might be OK because it's a link not a compilation warning. I'll test it and raise a separate PR if it works. |
…but the target was not found" in othe projects
…but the target was not found" in othe projects
Adds a step to the Debian-10 github workflow to install the CMake project and test that the examples can be built. A few fixes were necessary to get this to work.
Removed the
complex_io.cppexample as it failed to build with what looked like a non-trivial linking error, though someone with more familiarity with the code may be able to fix it.Resolved some clang warnings by removing deprecated usages (usually unecessary declarations of default copy constructors - see https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated-copy)
Added github workflow badge to Readme