-
-
Notifications
You must be signed in to change notification settings - Fork 326
Increment minor version: set version 2.1.0. #6028
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
base: develop
Are you sure you want to change the base?
Conversation
| @@ -1,4 +1,4 @@ | |||
| HDF5 version 2.0.1 currently under development | |||
| HDF5 version 2.1.0 currently under development | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need the "currently under development"? It is evident from examining the current release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line appears at the top of the CHANGELOG.md file and the top-level README.md file. The release script would change it to HDF5 version 2.1.0 released on 2026-05-15. (whatever the release date would be.
If the release date is not needed in downloaded code (and maybe binaries) is not needed, and the version is not needed the line can be removed from these files. (CHANGELOG.md currently has the version in the Executive Summary section.)
The version number and the release date could also be moved elsewhere in the README.md file.
Alternatively, the first half of the string could be kept and the release date added to it when the release occurs.
| * Short version string | ||
| */ | ||
| #define H5_VERS_STR "2.0.1" | ||
| #define H5_VERS_STR "2.1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we use a single set of #define H5_VERS_* and utilize those in the remaining #defines instead of the version number being hardcoded in multiple places across the various flavors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we just keep the H5_VERS_(MAJOR/MINOR/RELEASE) macros, it should be straightforward to form the others from those
|
I would much rather we move to a single source of truth for version numbers, rather than our current situation. A lot of this could be addressed by using templated files when we need version information. |
| # the add_subdirectory command.. | ||
| macro (EXTERNAL_HDF5_LIBRARY compress_type) | ||
| set (HDF5_VERSION "2.0.1") | ||
| set (HDF5_VERSION "2.1.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't CMake already have, or can it get, this information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit awkward, but in this case HDF5_VERSION may not be available as the library is being obtained with FetchContent. However, the only use seems to be in a status message that could be printed later anyway, so this occurrence could probably be removed.
| set (HDF5_VERSION "2.0.1") | ||
| set (HDF5_VERSION "2.1.0") | ||
| set (HDF5_VERSEXT "") | ||
| set (HDF5_VERSION_MAJOR "2.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if it's not removed, it looks like HDF5_VERSION_MAJOR needs to be updated anyway. Plus the naming doesn't make much sense, as it includes both the major and minor versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this is a carryover of CMake code from pre-2.0.0 days. HDF5_VERSION_MAJOR is set to ${HDF5_PACKAGE_VERSION_MAJOR} which is set to ${H5_VERS_MAJOR}.${H5_VERS_MINOR}.
There are no occurrences of HDF5_PACKAGE_VERSION_RELEASE nor of HDF5_VERSION_RELEASE.
Prior to 2.0 the major version was 1.x and there are 101 comparisons of HDF5_VERSION_MAJOR to versions "1.8" through "2.0". 16 of those are with "2.0". Would it be appropriate and improvements to replace them?
HDF5_PACKAGE_VERSION_MAJOR => HDF5_PACKAGE_VERSION_MAJ_MIN
HDF5_VERSION_MAJOR => HDF5_VERSION_MAJ_MIN
HDF5_VERSION_MINOR is set to ${HDF5_PACKAGE_VERSION_MINOR}, but neither are used except for in config/README.md.cmake.in where it appears they are used incorrectly (once in the line that creates the incorrect link that currently causes linkchecker to fail). Maybe these 2 should be removed.
HDF5_PACKAGE_VERSION_RELEASE and HDF5_VERSION_RELEASE don't exist.
brtnfld
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
New Fortran SWMR APIs were added; next release must increment minor version.
#6024
Important
Incremented HDF5 library version from 2.0.1 to 2.1.0 across documentation, configuration, and Java files.
2.0.1to2.1.0inREADME.md,HDF5config.cmake, andH5.java.LIB_VERSIONarray inH5.javaandTestH5.javato{2, 1, 0}.H5_VERS_MINORto1andH5_VERS_RELEASEto0inH5public.h.CHANGELOG.mdto2.1.0.This description was created by
for 4151e96. You can customize this summary. It will automatically update as commits are pushed.