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

Remove default parameters from detail headers in include #12888

Merged
merged 24 commits into from
Mar 13, 2023
Merged

Remove default parameters from detail headers in include #12888

merged 24 commits into from
Mar 13, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Mar 6, 2023

Description

Contributes to #9854. None of these changes should affect users, nor do they impose a particularly onerous burden on libcudf developers (just some extra passing through mr or cudf::get_default_stream().

Checklist

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

@vyasr vyasr added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 6, 2023
@vyasr vyasr self-assigned this Mar 6, 2023
@vyasr vyasr requested a review from a team as a code owner March 6, 2023 19:08
@vyasr vyasr requested review from bdice and mythrocks March 6, 2023 19:08
davidwendt
davidwendt requested changes Mar 6, 2023
View reviewed changes
Copy link
Contributor

@davidwendt davidwendt left a comment

Choose a reason for hiding this comment

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

Looks good.
Some issues with using mr variable for temporary objects.

cpp/src/copying/scatter.cu Outdated Show resolved Hide resolved
cpp/src/sort/segmented_sort_impl.cuh Outdated Show resolved Hide resolved
cpp/src/stream_compaction/distinct.cu Outdated Show resolved Hide resolved
cpp/src/stream_compaction/stable_distinct.cu Outdated Show resolved Hide resolved
@ttnghia ttnghia mentioned this pull request Mar 7, 2023
Merged
@vyasr vyasr requested a review from davidwendt March 7, 2023 20:51
davidwendt
davidwendt requested changes Mar 8, 2023
View reviewed changes
cpp/src/sort/segmented_sort_impl.cuh Outdated Show resolved Hide resolved
@vyasr vyasr requested a review from davidwendt March 8, 2023 21:00
davidwendt
davidwendt requested changes Mar 8, 2023
View reviewed changes
Copy link
Contributor

@davidwendt davidwendt left a comment

Choose a reason for hiding this comment

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

Looks good. Just some #include fixes.

cpp/src/stream_compaction/distinct_count.cu Outdated Show resolved Hide resolved
cpp/src/stream_compaction/unique_count.cu Outdated Show resolved Hide resolved
cpp/src/sort/segmented_sort_impl.cuh Outdated Show resolved Hide resolved
@vyasr vyasr requested a review from a team as a code owner March 9, 2023 22:34
@github-actions github-actions bot added the Java Affects Java cuDF API. label Mar 9, 2023
@vyasr vyasr requested a review from a team as a code owner March 9, 2023 23:14
@github-actions github-actions bot added the conda label Mar 9, 2023
@vyasr vyasr force-pushed the feat/remove_unnecessary_default_parameters branch from 728ae6b to 146afb2 Compare March 9, 2023 23:14
@vyasr vyasr removed the request for review from a team March 9, 2023 23:14
@github-actions github-actions bot removed the conda label Mar 9, 2023
@vyasr vyasr requested a review from davidwendt March 10, 2023 16:44
@vyasr vyasr mentioned this pull request Mar 13, 2023
Open
davidwendt
davidwendt reviewed Mar 13, 2023
View reviewed changes
cpp/src/copying/copy.cu Outdated Show resolved Hide resolved
@vyasr
Copy link
Contributor Author

vyasr commented Mar 13, 2023

/merge

@rapids-bot rapids-bot bot merged commit 9c9dd54 into rapidsai:branch-23.04 Mar 13, 2023
@vyasr vyasr deleted the feat/remove_unnecessary_default_parameters branch March 13, 2023 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidwendt davidwendt davidwendt approved these changes

@ttnghia ttnghia ttnghia approved these changes

@bdice bdice Awaiting requested review from bdice bdice is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

@mythrocks mythrocks Awaiting requested review from mythrocks mythrocks is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@vyasr vyasr

Labels
3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vyasr @bdice @ttnghia @davidwendt