-
Notifications
You must be signed in to change notification settings - Fork 934
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
Read version from VERSION file in CMake #14867
Read version from VERSION file in CMake #14867
Conversation
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.
Need to also change https://github.com/rapidsai/cudf/blob/branch-24.02/ci/release/update-version.sh
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.
Can we remove the lines from ci/release/update-version.sh
that update files that are now templated on this variable?
edit: Rob was faster :D
This should target branch-24.04, since 24.02 is in burndown (code freeze starts tomorrow). |
830aac5
to
73c1b28
Compare
Can you provide PR description please? |
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.
Great change! Is there an issue tracking this change for all RAPIDS repositories?
None that I know of. I'd also like to add a linter to https://github.com/rapidsai/pre-commit-hooks that warns you if you hard-code a version number (or updates it if the file is configured to allow hard-coding.) Edit: here's the comment I posted that promped this: https://github.com/rapidsai/ops/issues/2549#issuecomment-1908638851 |
Should most likely create a tracking issue on https://github.com/rapidsai/build-planning/ |
I've created rapidsai/build-planning#15. |
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.
Java approval
Looks like there is a merge conflict with |
/merge |
Description
Rather than hard-coding the RAPIDS version throughout CMake code, have a single CMake module that reads it from
VERSION
and provides it as a variable.Checklist