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

Read version from VERSION file in CMake #14867

Merged
merged 12 commits into from
Feb 22, 2024
Merged

Read version from VERSION file in CMake #14867

merged 12 commits into from
Feb 22, 2024

Conversation

KyleFromNVIDIA
Copy link
Contributor

@KyleFromNVIDIA KyleFromNVIDIA commented Jan 24, 2024 •
edited
Loading

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

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@KyleFromNVIDIA KyleFromNVIDIA requested review from a team as code owners January 24, 2024 18:38
@github-actions github-actions bot added libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. CMake CMake build issue Java Affects Java cuDF API. labels Jan 24, 2024
@KyleFromNVIDIA KyleFromNVIDIA added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. Java Affects Java cuDF API. and removed libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. Java Affects Java cuDF API. labels Jan 24, 2024
robertmaynard
robertmaynard requested changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@robertmaynard robertmaynard left a 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

bdice
bdice reviewed Jan 24, 2024
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment •
edited
Loading

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

rapids_version.cmake Outdated Show resolved Hide resolved
@bdice
Copy link
Contributor

bdice commented Jan 24, 2024 •
edited
Loading

This should target branch-24.04, since 24.02 is in burndown (code freeze starts tomorrow).

@KyleFromNVIDIA KyleFromNVIDIA requested review from a team as code owners January 24, 2024 18:47
@KyleFromNVIDIA KyleFromNVIDIA changed the base branch from branch-24.02 to branch-24.04 January 24, 2024 18:48
@github-actions github-actions bot removed the conda label Jan 24, 2024
@ttnghia
Copy link
Contributor

ttnghia commented Jan 24, 2024

Can you provide PR description please?

bdice
bdice reviewed Jan 24, 2024
View reviewed changes
rapids_config.cmake Outdated Show resolved Hide resolved
bdice
bdice approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@bdice bdice left a 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?

robertmaynard
robertmaynard requested changes Jan 24, 2024
View reviewed changes
rapids_config.cmake Show resolved Hide resolved
@KyleFromNVIDIA
Copy link
Contributor Author

KyleFromNVIDIA commented Jan 24, 2024 •
edited
Loading

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

@robertmaynard
Copy link
Contributor

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: rapidsai/ops#2549 (comment)

Should most likely create a tracking issue on https://github.com/rapidsai/build-planning/

@KyleFromNVIDIA KyleFromNVIDIA mentioned this pull request Jan 24, 2024
Closed
@KyleFromNVIDIA
Copy link
Contributor Author

I've created rapidsai/build-planning#15.

jlowe
jlowe approved these changes Feb 9, 2024
View reviewed changes
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Java approval

@jakirkham
Copy link
Member

jakirkham commented Feb 9, 2024 •
edited
Loading

Looks like there is a merge conflict with fetch_rapids.cmake

@vyasr
Copy link
Contributor

vyasr commented Feb 22, 2024

/merge

@rapids-bot rapids-bot bot merged commit 296185c into rapidsai:branch-24.04 Feb 22, 2024
68 checks passed
@bdice bdice mentioned this pull request Mar 4, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robertmaynard robertmaynard robertmaynard approved these changes

@jlowe jlowe jlowe approved these changes

@raydouglass raydouglass raydouglass approved these changes

@bdice bdice bdice approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

@charlesbluca charlesbluca charlesbluca approved these changes

@hyperbolic2346 hyperbolic2346 Awaiting requested review from hyperbolic2346 hyperbolic2346 was automatically assigned from rapidsai/cudf-cpp-codeowners

@shrshi shrshi Awaiting requested review from shrshi shrshi was automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees
No one assigned
Labels
CMake CMake build issue improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
cuDF/Dask/Numba/UCX
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@KyleFromNVIDIA @bdice @ttnghia @robertmaynard @jakirkham @vyasr @jlowe @raydouglass @galipremsagar @charlesbluca