Skip to content
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

CMake P4EST_BUILD_EXAMPLES flag #316

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

Conversation

dutkalex
Copy link

CMake P4EST_BUILD_EXAMPLES flag

Just adds this flag to give the opportunity to downstream projects to disable this and save on build times.

@cburstedde
Copy link
Owner

Thanks! We had previously set examples to be always built, but it's fine to add this option as you suggest with default ON. While we're at it:

  • Can we make an analogous change in libsc, double-checking that the CMake example logic looks the exact same in both libraries?
  • I would not build the tests by default, only on make check, as with autoconf. Is this possible with CMake (and again, for both p4est and sc)?

@dutkalex
Copy link
Author

  • Can we make an analogous change in libsc, double-checking that the CMake example logic looks the exact same in both libraries?

Yes that seems reasonable. I can open a quick PR on libsc too.

  • I would not build the tests by default, only on make check, as with autoconf. Is this possible with CMake (and again, for both p4est and sc)?

That I do not know. Maybe it can be done, but no obvious solution comes to my mind right now...

@cburstedde
Copy link
Owner

  • Can we make an analogous change in libsc, double-checking that the CMake example logic looks the exact same in both libraries?

Yes that seems reasonable. I can open a quick PR on libsc too.

Sounds great! There seems to be just some small glitch with the CI. And please add your author file to the docs, and a line to the doc/release_notes.txt

  • I would not build the tests by default, only on make check, as with autoconf. Is this possible with CMake (and again, for both p4est and sc)?

That I do not know. Maybe it can be done, but no obvious solution comes to my mind right now...

Thanks; then we let this remain as is for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants