Skip to content

ARROW-13974: [C++] Resolve follow-up reviews for TopK/BottomK - #11132

Closed
aocsa wants to merge 2 commits into
apache:masterfrom
aocsa:ARROW-13974
Closed

ARROW-13974: [C++] Resolve follow-up reviews for TopK/BottomK#11132
aocsa wants to merge 2 commits into
apache:masterfrom
aocsa:ARROW-13974

Conversation

@aocsa

@aocsaaocsa commented Sep 10, 2021

Copy link
Copy Markdown
Contributor

There are some follow-up reviews for topK/bottomK kernel (ARROW-1565) that were requested after PR was closed. This issue is to resolve them.

@github-actions

github-actionsBot commented Sep 10, 2021

Copy link
Copy Markdown

https://issues.apache.org/jira/browse/ARROW-13974

@github-actions

github-actionsBot commented Sep 10, 2021

Copy link
Copy Markdown

⚠️ Ticket has no components in JIRA, make sure you assign one.

@aocsa

aocsa commented Sep 10, 2021

Copy link
Copy Markdown
ContributorAuthor

Thanks @edponce, I addressed your feedback comments. Let me know if anything else needs to be taken care of to merge these minor updates. cc @lidavidm

lidavidm
lidavidm reviewed Sep 10, 2021
View reviewed changes
Comment threadcpp/src/arrow/compute/kernels/select_k_test.cc Outdated
Comment threadcpp/src/arrow/compute/api_vector.h Outdated
edponce
edponce reviewed Sep 10, 2021
View reviewed changes
Comment threadcpp/src/arrow/compute/kernels/vector_sort.cc
Comment threadcpp/src/arrow/compute/kernels/vector_sort.cc
edponce
edponce reviewed Sep 10, 2021
View reviewed changes
Comment threadcpp/src/arrow/compute/kernels/select_k_test.cc
@edponce

edponce commented Sep 10, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

LGTM. Thanks for working on this @aocsa!

@aocsa

aocsa commented Sep 10, 2021

Copy link
Copy Markdown
ContributorAuthor

Thanks @edponce, @lidavidm
I updated with minor fixes addressing your comments. I think it is ready for merging.

lidavidm
lidavidm approved these changes Sep 10, 2021
View reviewed changes

@lidavidmlidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you! CC @edponce

ViniciusSouzaRoque pushed a commit to s1mbi0se/arrow that referenced this pull request Oct 20, 2021
@aocsa@ViniciusSouzaRoque
ARROW-13974: [C++] Resolve follow-up reviews for TopK/BottomK
5e9c565
There are some follow-up reviews for topK/bottomK kernel (ARROW-1565) that were requested after PR was closed. This issue is to resolve them.
Closesapache#11132 from aocsa/ARROW-13974
Authored-by: Alexander <aocsa.cs@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@asfimportasfimport mentioned this pull request Sep 13, 2021
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lidavidmlidavidmlidavidm approved these changes
+1 more reviewer
@edponceedponceedponce left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aocsa@edponce@lidavidm