-
-
Notifications
You must be signed in to change notification settings - Fork 290
Build examples only if master project by default #321
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
Conversation
|
Personally not inclined to this, as you mentioned it was easy for you to set this on your end. My reasoning is users like #319 which might not know so it best to have examples compiled by default to help find problems. Maybe if it was a single line or was more consistent in the ecosystem I would be swayed. This doesn't help for package managers either so it's a really slim user. Open to my mind being changed! |
|
Honestly, I don't think a new user will need to compile the examples to understand how to use this library. Source codes usually help with this. Maybe at least add a warning about this in the README? I mean instructions on how to disable example compilation? This won't help people who know about it (like our team), it can only help new users. |
good suggestion Thalhammer#321 (comment)
Ahh, I meant specific that it can build, because without anything to compile (being a header only project) its hard to know if the setup was correct. By having examples enabled by default, comments like this..
..have a good amount, of information. Now that's not to say a CMake novice might be using the wrong words. My hope is that by making new users build them we know the system in properly configured (which is very often a pain point). Hopefully that clarifies my last remarks 😄 |
|
Sure, but using Lines 21 to 22 in 953aab6
I do think improving the documentation in this area is super important and I've included your suggestion as I've been working on this. However looking at my copy of Professional CMake 16th edition, I think I've talked myself in this is a good change, I am going to poke some friends and colleagues to get more opinions. |
* Create getting-started.md * Update getting-started.md * Create header-only.yml * Create package-managers.yml * Update package-managers.yml * Update getting-started.md * Update getting-started.md * Update header-only.yml * Update header-only.yml * Update header-only.yml * Update header-only.yml * use `jwt::algorithm::hs256` to link against openssl * add crypto * switch to ssl * Update header-only.yml * tidy * lets install conan * tidying * Adding a CMake target to make the docs * cleaning up repo root folder * fix typo * set docs flag as on * update awesome doxy theme to latest * Updated overrides for new version * output folder and target name are changed * add a few more docs * white space * cmake format * playing with settings * move algorithms - magically fixed missing structs * playing with the idea of more refactoring * more testing * moved evp_handle def to special namespace for doxygen * split evp class by it's self * fix doc links * playing with evp class to fix error -- no luck * revert testing changes * clang format * cache should be container specific * clean up includes * combined class definition * split algos but running in ci with older version * trying slightly newer version of doxygen * doxygen 196 * doxygen 197 * doxygen 198 * avoid regression in 198 * more docs * playing with adding examples * playing around with example support to see if I like * docs include all the example files to be referenced in the code * bump version to 0.7.0 * missing header * devcontainer for testing openssl 3 * add test explorer * add clang helpers * there's a lot of package managers in 2023 * fixup colors with doxygen 1.10 * bump doxygen version since my bug has been fixed * move everything to one file (too many changes) * update doxyfile to 1.10 * minimize diff * fix extra line * fix whitespace * clean up dev container * min changes * cleanup * better checking + warnings for doxygen * whitespace * adding in more docs * fix spelling * make sure traits appear * trying to add snippets from examples cant figure it out doxygen/doxygen#10517 * Update faqs.md * Delete .github/workflows/header-only.yml * Delete .github/workflows/package-managers.yml * Delete example/conan/CMakeLists.txt * Delete example/conan/conanfile.txt * Delete example/conan/main.cpp * Delete example/conan/README.md * Update nlohmann-json.cpp * add updated getting started docs first pass towards #319 * touch ups * linking deprecated messages * improve cmake with find_package examples * fix code link color * be more clear about turning off examples good suggestion #321 (comment) * clarify openssl install and fixup fetch tag * apply workaround from doxygen/doxygen#10517 (comment) * fix renamed types * linting * cross reference new docs * trying new alert syntax https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#alerts * move sections out of readme trying to make it shorter and more approachable * improve "getting started" readme section with a more through example * sort out doubled "getting started" second one in docs/ is just installation * linter * fixup english * formatting * whitespace * tie in more examples with snippets * minor touch ups * Update signing.md * JWT_BUILD_DOCS * better cache string for new option * word crafting * word crafting * add missing open-source-parsers/jsoncpp to traits list * better github alert * fixup new shields io start style * improve docs cmake target name * linter --------- Co-authored-by: Christopher McArthur <christopherm@jfrog.com>
* Create getting-started.md * Update getting-started.md * Create header-only.yml * Create package-managers.yml * Update package-managers.yml * Update getting-started.md * Update getting-started.md * Update header-only.yml * Update header-only.yml * Update header-only.yml * Update header-only.yml * use `jwt::algorithm::hs256` to link against openssl * add crypto * switch to ssl * Update header-only.yml * tidy * lets install conan * tidying * Adding a CMake target to make the docs * cleaning up repo root folder * fix typo * set docs flag as on * update awesome doxy theme to latest * Updated overrides for new version * output folder and target name are changed * add a few more docs * white space * cmake format * playing with settings * move algorithms - magically fixed missing structs * playing with the idea of more refactoring * more testing * moved evp_handle def to special namespace for doxygen * split evp class by it's self * fix doc links * playing with evp class to fix error -- no luck * revert testing changes * clang format * cache should be container specific * clean up includes * combined class definition * split algos but running in ci with older version * trying slightly newer version of doxygen * doxygen 196 * doxygen 197 * doxygen 198 * avoid regression in 198 * more docs * playing with adding examples * playing around with example support to see if I like * docs include all the example files to be referenced in the code * bump version to 0.7.0 * missing header * devcontainer for testing openssl 3 * add test explorer * add clang helpers * there's a lot of package managers in 2023 * fixup colors with doxygen 1.10 * bump doxygen version since my bug has been fixed * move everything to one file (too many changes) * update doxyfile to 1.10 * minimize diff * fix extra line * fix whitespace * clean up dev container * min changes * cleanup * better checking + warnings for doxygen * whitespace * adding in more docs * fix spelling * make sure traits appear * trying to add snippets from examples cant figure it out doxygen/doxygen#10517 * Update faqs.md * Delete .github/workflows/header-only.yml * Delete .github/workflows/package-managers.yml * Delete example/conan/CMakeLists.txt * Delete example/conan/conanfile.txt * Delete example/conan/main.cpp * Delete example/conan/README.md * Update nlohmann-json.cpp * add updated getting started docs first pass towards Thalhammer#319 * touch ups * linking deprecated messages * improve cmake with find_package examples * fix code link color * be more clear about turning off examples good suggestion Thalhammer#321 (comment) * clarify openssl install and fixup fetch tag * apply workaround from doxygen/doxygen#10517 (comment) * fix renamed types * linting * cross reference new docs * trying new alert syntax https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#alerts * move sections out of readme trying to make it shorter and more approachable * improve "getting started" readme section with a more through example * sort out doubled "getting started" second one in docs/ is just installation * linter * fixup english * formatting * whitespace * tie in more examples with snippets * minor touch ups * Update signing.md * JWT_BUILD_DOCS * better cache string for new option * word crafting * word crafting * add missing open-source-parsers/jsoncpp to traits list * better github alert * fixup new shields io start style * improve docs cmake target name * linter --------- Co-authored-by: Christopher McArthur <christopherm@jfrog.com>

Hi there!
I changed the default value of JWT_BUILD_EXAMPLES to true iff jwt is the main project.
We are using jwt-cpp as a submodule, and although we have manually disabled this option, it is strange to build examples in a case with a submodule where this is not explicitly set.
What do you think?