Skip to content

Conversation

@shivusondur
Copy link
Contributor

What changes were proposed in this pull request?

removed the unused "UnsafeKeyValueSorter.java" file

How was this patch tested?

Ran Compilation and UT locally.

Please review http://spark.apache.org/contributing.html before opening a pull request.

@HyukjinKwon
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented May 16, 2019

Test build #105435 has finished for PR 24622 at commit 0a2b132.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivusondur
Copy link
Contributor Author

please retest this

@shivusondur shivusondur reopened this May 16, 2019
@shivusondur
Copy link
Contributor Author

@HyukjinKwon
Test Case failure is not relevant to this MR,
please retest

@viirya
Copy link
Member

viirya commented May 16, 2019

retest this please.

@cloud-fan
Copy link
Contributor

ok to test

Copy link
Member

@HyukjinKwon HyukjinKwon 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 if build and tests pass. I think there's no point of keeping unused private codes in Spark - it just makes us hard to read the codes.

@SparkQA
Copy link

SparkQA commented May 16, 2019

Test build #105443 has finished for PR 24622 at commit f3f9244.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

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

The build and tests verify this class is not used. If no special reason to keep it, I think it is fine to remove it.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in c6a45e6 May 16, 2019
@shivusondur
Copy link
Contributor Author

shivusondur commented May 16, 2019

@HyukjinKwon , @cloud-fan , @viirya
thanks for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants