-
Notifications
You must be signed in to change notification settings - Fork 163
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
Include H5Version.hpp
in the sources.
#819
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #819 +/- ##
=========================================
Coverage ? 84.59%
=========================================
Files ? 68
Lines ? 4784
Branches ? 0
=========================================
Hits ? 4047
Misses ? 737
Partials ? 0 ☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here. |
92e1aa0
to
c417e6a
Compare
Here the new CI ran with an outdated |
c417e6a
to
3b5de76
Compare
3b5de76
to
06b80e0
Compare
run: | | ||
# Check that the file hasn't changed, i.e. was updated | ||
# after changing the version number. | ||
! git status | grep include/highfive/H5Version.hpp |
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.
Would be nice if we could ensure that the version is also greater or equal to the latest tag.
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 think understand the motivation. It would be nice to have something that checks we didn't forget to bump the version number.
Our current workflow is to:
- Create a commit which updates the change log and bumps the version.
- Create a PR.
- Merge the PR.
- Tag the appropriate commit on
master
.
Therefore a naive solution has two issues:
- There's a brief period where the version number is strictly larger than the latest tag. Which requires as you say, that we check for greater or equal.
- Due to the greater or equal it'll initially succeed even if we forget to bump the version number. Only after we tag a commit, will it complain. That too late for my taste.
Instead it would be nicer to have a CD pipeline to create a release, it will change the version in CMakeLists.txt
and tag consistently.
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.
Instead it would be nicer to have a CD pipeline to create a release, it will change the version in CMakeLists.txt and tag consistently.
Yes, agreed. We can put this in a virtual backlog, let's get this in first.
Generated files require users to "build" and "install" HighFive, for the sole purpose of having files generated via
configure_file
available. The only file that generated in HighFive is the version file.This PR proposes to automatically generate the file but also commit it to the repo. The advantage of doing so, is that users can clone the repo, set the appropriate value for
-I
and they're done (if they don't need any addons).In order to check that the version number in
CMakeLists.txt
andH5Version.hpp
are consistent we configure HighFive during CI and check that the file hasn't changed.