Skip to content
This repository was archived by the owner on Oct 17, 2019. It is now read-only.

Fix up includes to nlohmann json - #448

Open
z3ntu wants to merge 2 commits into
mujx:masterfrom
z3ntu:nlohmann_json
Open

Fix up includes to nlohmann json#448
z3ntu wants to merge 2 commits into
mujx:masterfrom
z3ntu:nlohmann_json

Conversation

@z3ntu

@z3ntu z3ntu commented Sep 24, 2018

Copy link
Copy Markdown

Might require more changes for the changed include path

See also mujx/mtxclient#29

@z3ntu

z3ntu commented Sep 24, 2018

Copy link
Copy Markdown
Author

The build fails because the mtxclient PR isn't used in the build process.

Comment thread deps/CMakeLists.txt
ce6b5610a051ec6795fa11c33854abebb086f0fd67c311f5921c3c07f9531b44)

file(DOWNLOAD ${JSON_HEADER_URL} ${DEPS_INSTALL_DIR}/include/json.hpp
file(DOWNLOAD ${JSON_HEADER_URL} ${DEPS_INSTALL_DIR}/include/nlohmann/json.hpp

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to use External_Project for this library and install it like the other dependencies. And then call find_package(nlohmann_json) which will fix the CI.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably but it's more work 😉 I'll look into it soon-ish

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants