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

Fix up includes to nlohmann json #448

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix up includes to nlohmann json #448

wants to merge 2 commits into from

Conversation

z3ntu
Copy link

@z3ntu z3ntu commented Sep 24, 2018

Might require more changes for the changed include path

See also mujx/mtxclient#29

@z3ntu
Copy link
Author

z3ntu commented Sep 24, 2018

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

@@ -73,7 +73,7 @@ set(JSON_HEADER_URL
set(JSON_HEADER_HASH
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
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
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