search

ArthurSonzogni / nlohmann_json_cmake_fetchcontent

A lightweight Release-tracking repository for nlohmann/json. Suitable for CMake fetchcontent. Automatically upgraded every weeks.
MIT License
79 stars 25 forks source link

Add some doc to README and format it according to markdownlint rules #10

Closed NicolasIRAGNE closed 2 years ago

NicolasIRAGNE commented 2 years ago

This is pretty minor so opening another PR feels a bit overkill but is there any reason for not using FetchContent_MakeAvailable (i.e. in the README)? This seems like the recommended approach. See the CMake docs about FetchContent.

I am guessing it is because of the EXCLUDE_FROM_ALL property, which is a bit more intricate to use with Fetchcontent_MakeAvailable (see https://gitlab.kitware.com/cmake/cmake/-/issues/20167). Is this problematic?

EDIT: I got a bit carried away and rewrote some parts of the README. I think it is more explicit now, but this is very subjective and I'd understand if you prefer the previous one :-)

ArthurSonzogni commented 2 years ago

Thanks! I like it. I will merge it tonight just before the other PR.

NicolasIRAGNE commented 2 years ago

By the way, I just thought of something: should this repository support JSON_MultipleHeaders, or do we then leave the area of lightweight? From what I understand, this would just imply copying both include directories inside the update script, yes?

I personally use the single header and it is probably the general use case but should it be included for completeness?

ArthurSonzogni commented 2 years ago

I think I would prefer not to support multiple header, to keep this lightweight.