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

Rework the compression library search #5085

Merged
merged 38 commits into from
Dec 13, 2024

Conversation

byrnHDF
Copy link
Contributor

@byrnHDF byrnHDF commented Nov 8, 2024

Added default names for search use.

@byrnHDF byrnHDF added Priority - 2. Medium ⏹ It would be nice to have this in the next release Component - C Library Core C library issues (usually in the src directory) Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub labels Nov 8, 2024
@byrnHDF byrnHDF self-assigned this Nov 8, 2024
@byrnHDF
Copy link
Contributor Author

byrnHDF commented Nov 20, 2024

The search path for libs can be controlled by CMake system variables - see https://cmake.org/cmake/help/latest/command/find_package.html

CMakeFilters.cmake Outdated Show resolved Hide resolved
@@ -68,7 +68,7 @@ Download from https://github.com/HDFGroup/hdf5/blob/develop/config/cmake/scripts
External compression plugin libraries from https://github.com/HDFGroup/hdf5_plugins:
hdf5_plugins.tar.gz

External compression szip and zlib libraries:
External compression szip and zlib libraries:0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Accidental?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most likely yes.

@@ -50,7 +50,7 @@ New Features
- Renamed the option: HDF5_ENABLE_Z_LIB_SUPPORT

The option has been renamed to HDF5_ENABLE_ZLIB_SUPPORT to be consistent
with the naming of other options.
with the naming of other options. Also, the option defaults to OFF.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Definitely want to make sure this is very well called out for the next release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely

lrknox
lrknox previously approved these changes Dec 2, 2024
@@ -3,7 +3,13 @@ HDF5 version 2.0.0 currently under development
Features included for the next major release:
----------------------------------------------------------------------------

*
************ Renamed the option: HDF5_ENABLE_Z_LIB_SUPPORT ************
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we break this into two items, with one indicating the rename and one indicating that it now defaults to off? We'd want someone skimming the headlines to catch both of these


The option has been renamed to HDF5_ENABLE_ZLIB_SUPPORT to be consistent
with the naming of other options.
*** Also, the option defaults to OFF. This requires the user to explicitly ***
Copy link
Contributor

Choose a reason for hiding this comment

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

Same idea about breaking this into two items

@brtnfld
Copy link
Contributor

brtnfld commented Dec 2, 2024

I suggest we transition from using txt files to markdown, enabling us to highlight important information more effectively by the use of:

https://github.blog/changelog/2023-12-14-new-markdown-extension-alerts-provide-distinctive-styling-for-significant-content/

lrknox
lrknox previously approved these changes Dec 5, 2024
lrknox
lrknox previously approved these changes Dec 11, 2024
jhendersonHDF
jhendersonHDF previously approved these changes Dec 13, 2024
@byrnHDF byrnHDF dismissed stale reviews from jhendersonHDF and lrknox via 318b4d2 December 13, 2024 16:01
@lrknox lrknox merged commit 1a9f13a into HDFGroup:develop Dec 13, 2024
68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - C Library Core C library issues (usually in the src directory) Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants