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.

Sign up for GitHub

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

Jump to bottom

Don't build util tests when HDF5_EXTERNALLY_CONFIGURED=ON #3781

Merged
merged 2 commits into from
Oct 30, 2023
Merged

Don't build util tests when HDF5_EXTERNALLY_CONFIGURED=ON #3781

merged 2 commits into from
Oct 30, 2023

Conversation

Krzmbrzl
Copy link
Contributor

Fixes #3780

Krzmbrzl
Krzmbrzl commented Oct 26, 2023
View reviewed changes
utils/CMakeLists.txt
Comment on lines +4 to 6
if (NOT HDF5_EXTERNALLY_CONFIGURED AND BUILD_TESTING)
add_subdirectory (test)
endif ()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one could also think of only building the tests if the utils are built in the first place. Building only the tests doesn't seem to make much sense 🤔 👀

byrnHDF
byrnHDF requested changes Oct 26, 2023
View reviewed changes
Copy link
Contributor

@byrnHDF byrnHDF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should guard all the add_subdirectory test folders, not just an arbitrary instance.

@Krzmbrzl Krzmbrzl requested a review from epourmal as a code owner October 26, 2023 14:31
@Krzmbrzl
Copy link
Contributor Author

I think I added guards for all other tests as well 👀

@lrknox lrknox merged commit 87b3e7c into HDFGroup:develop Oct 30, 2023
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byrnHDF byrnHDF byrnHDF approved these changes

@jhendersonHDF jhendersonHDF jhendersonHDF approved these changes

@lrknox lrknox lrknox approved these changes

@brtnfld brtnfld brtnfld approved these changes

@derobins derobins Awaiting requested review from derobins derobins is a code owner

@fortnern fortnern Awaiting requested review from fortnern fortnern is a code owner

@qkoziol qkoziol Awaiting requested review from qkoziol qkoziol is a code owner

@vchoi-hdfgroup vchoi-hdfgroup Awaiting requested review from vchoi-hdfgroup vchoi-hdfgroup is a code owner

@bmribler bmribler Awaiting requested review from bmribler bmribler is a code owner

@glennsong09 glennsong09 Awaiting requested review from glennsong09 glennsong09 is a code owner

@mattjala mattjala Awaiting requested review from mattjala mattjala is a code owner

@epourmal epourmal Awaiting requested review from epourmal epourmal is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can't compile with HDF5_EXTERNALLY_CONFIGURED=ON (cmake)
5 participants
@Krzmbrzl @brtnfld @lrknox @jhendersonHDF @byrnHDF