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

Restore changes from #653 and #655 and correct cmake component dependencies #686

Merged
merged 6 commits into from
May 27, 2022
Merged

Restore changes from #653 and #655 and correct cmake component dependencies #686

merged 6 commits into from
May 27, 2022

Conversation

robertmaynard
Copy link
Contributor

No description provided.

@robertmaynard robertmaynard added 3 - Ready for Review feature request New feature or request non-breaking Non-breaking change labels May 25, 2022
@robertmaynard robertmaynard requested a review from a team as a code owner May 25, 2022 20:59
@vyasr
Copy link
Contributor

vyasr commented May 25, 2022

Was something wrong with #668?

@robertmaynard
Copy link
Contributor Author

Was something wrong with #668?

I felt a new PR was needed as this isn't just a pure revert, but also changing the behavior of dependency recording.
But if we want me to instead push to #668 I would be happy to.

@vyasr
Copy link
Contributor

vyasr commented May 25, 2022

That's fine, I was just confused to see this.

@cjnolet
Copy link
Member

cjnolet commented May 25, 2022

@vyasr I opened #668 just out of courtesy since I was the one that did the original revert last week. I certainly have no problem at all with a new PR if it helps us get closer to merging #641.

vyasr
vyasr reviewed May 25, 2022
View reviewed changes
cpp/CMakeLists.txt Show resolved Hide resolved
cpp/CMakeLists.txt Outdated Show resolved Hide resolved
cpp/cmake/modules/raft_export.cmake Outdated Show resolved Hide resolved
robertmaynard and others added 2 commits May 26, 2022 08:43
@robertmaynard @vyasr
Update cpp/CMakeLists.txt
8befd20 
Co-authored-by: Vyas Ramasubramani <vyas.ramasubramani@gmail.com>
@robertmaynard
refactor placement to remove need for an extra install command
2c95382
vyasr
vyasr reviewed May 26, 2022
View reviewed changes
cpp/CMakeLists.txt Outdated Show resolved Hide resolved
cpp/CMakeLists.txt Show resolved Hide resolved
@robertmaynard robertmaynard requested a review from vyasr May 27, 2022 15:38
vyasr
vyasr approved these changes May 27, 2022
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

One question, otherwise LGTM.

build.sh Show resolved Hide resolved
@vyasr
Copy link
Contributor

vyasr commented May 27, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 2450bdd into rapidsai:branch-22.06 May 27, 2022
@robertmaynard robertmaynard deleted the fea/enable_cmake_components branch November 1, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vyasr vyasr vyasr approved these changes

Assignees

@robertmaynard robertmaynard

Labels
3 - Ready for Review CMake cpp feature request New feature or request non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@robertmaynard @vyasr @cjnolet