Add cmake options for enabling and disabling builds

[kotaweav]

add cmake flags:

• BUILD_EXAMPLES: whether to build the examples
• BUILD_TESTS: whether to build the tests
• BUILD_BENCHMARKS: whether to build the benchmarks

these are all off by default.

[github-actions]

Hi there! Thank you for creating your first pull-request on the Graaf library :)

[bobluppes]

Hi @kotaweav, thanks for the contribution! I think adding such options to our build system is a great addition.

I am however thinking whether we should create an "opt-out" system for these executables rather than an "opt-in". What do you think about introducing options such as SKIP_EXAMPLES, SKIP_TESTS, and SKIP_BENCHMARKS which are all set to OFF by default?

That way existing build systems would keep working without any change. It might also prevent confusion when a developer changes a class, successfully compiles the project locally, but the CI fails because some executable (which the developer did not explicitly opt-in for) cannot be compiled anymore.

[bobluppes]

Hi @kotaweav, do you still intend to work on this?

[kotaweav]

Yup, sorry, I'm overseas for work at the moment. I'll do this as soon as I get back!

…

On Fri, Sep 8, 2023, 05:42 Bob Luppes ***@***.***> wrote: Hi @kotaweav <https://github.com/kotaweav>, do you still intend to work on this? — Reply to this email directly, view it on GitHub <#79 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEWLUOJ7SIGRQL2UDBDLN3XZIWUBANCNFSM6AAAAAA3SV4LQA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[kotaweav]

Hi @kotaweav, thanks for the contribution! I think adding such options to our build system is a great addition.

I am however thinking whether we should create an "opt-out" system for these executables rather than an "opt-in". What do you think about introducing options such as SKIP_EXAMPLES, SKIP_TESTS, and SKIP_BENCHMARKS which are all set to OFF by default?

That way existing build systems would keep working without any change. It might also prevent confusion when a developer changes a class, successfully compiles the project locally, but the CI fails because some executable (which the developer did not explicitly opt-in for) cannot be compiled anymore.

Hey! So, I'm back in town and wanted to revisit this. What are your thoughts instead of having BUILD_TESTS, etc. but ON by default? I think it would be less awkward to have it that way than to have them be double negatives? Would appreciate your thoughts! Thanks!

[bobluppes]

Hi @kotaweav, thanks for the contribution! I think adding such options to our build system is a great addition.
I am however thinking whether we should create an "opt-out" system for these executables rather than an "opt-in". What do you think about introducing options such as SKIP_EXAMPLES, SKIP_TESTS, and SKIP_BENCHMARKS which are all set to OFF by default?
That way existing build systems would keep working without any change. It might also prevent confusion when a developer changes a class, successfully compiles the project locally, but the CI fails because some executable (which the developer did not explicitly opt-in for) cannot be compiled anymore.

Hey! So, I'm back in town and wanted to revisit this. What are your thoughts instead of having BUILD_TESTS, etc. but ON by default? I think it would be less awkward to have it that way than to have them be double negatives? Would appreciate your thoughts! Thanks!

Hi, welcome back :)

My initial thought is that I would slightly prefer to express these flag as SKIP_* in order to signify it is an opt-out system. This would also be in line with how mvn handles this.

But I also do not have strong opinions, so I would be happy with both solutions. What do you think?

[kotaweav]

I've certainly seen a lot of both XD OpenCV for example, goes the ENABLE_ route: https://github.com/opencv/opencv/blob/4.x/CMakeLists.txt#L519

But sure, I'll go ahead with SKIP_! Thanks for putting together such a cool library! 😄

[codecov]

Codecov Report

All modified lines are covered by tests ✅

see 12 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

[bobluppes]

I've certainly seen a lot of both XD OpenCV for example, goes the ENABLE_ route: https://github.com/opencv/opencv/blob/4.x/CMakeLists.txt#L519

But sure, I'll go ahead with SKIP_! Thanks for putting together such a cool library! 😄

Haha, I am not sure if we will ever reach some standardization of such build configurations 😄 But thanks for addressing this!

[bobluppes]

LGTM!
Congrats on the first contribution to Graaf 🎉