Skip to content

[MINOR] Move java file to java directory - #24222

Closed
ConeyLiu wants to merge 3 commits into
apache:masterfrom
ConeyLiu:move-scala-to-java
Closed

[MINOR] Move java file to java directory#24222
ConeyLiu wants to merge 3 commits into
apache:masterfrom
ConeyLiu:move-scala-to-java

Conversation

@ConeyLiu

@ConeyLiuConeyLiu commented Mar 26, 2019

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

move

org.apache.spark.sql.execution.streaming.BaseStreamingSource
org.apache.spark.sql.execution.streaming.BaseStreamingSink

to java directory

How was this patch tested?

Existing UT.

@ConeyLiu

Copy link
Copy Markdown
ContributorAuthor

Hi @srowen, can you help to review. Thanks a lot.

@srowen

Copy link
Copy Markdown
Member

I agree with moving the code, but why change the package?

@srowen

Copy link
Copy Markdown
Member

Offset.java can be moved too?

@dongjoon-hyun

Copy link
Copy Markdown
Member

+1 for @srowen 's comment. We should keep the package name.

@dongjoon-hyun

Copy link
Copy Markdown
Member

ok to test

@SparkQA

Copy link
Copy Markdown

Test build #103982 has finished for PR 24222 at commit 5ae78b4.

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

@felixcheungfelixcheung changed the title [MINOR] Move java file to java foler[MINOR] Move java file to java directoryMar 27, 2019
@ConeyLiu

ConeyLiu commented Mar 27, 2019

Copy link
Copy Markdown
ContributorAuthor

thanks @srowen@dongjoon-hyun for the review. I am referring to @cloud-fan 's point of view.
shall we move these new interfaces to org.apache.spark.sql.sources.v2.reader/write.streaming package?

Offset.java can be moved too?

Will move it later.

@cloud-fan

Copy link
Copy Markdown
Contributor

These 2 interfaces are internal and I don't think we can move them to a public package now.

@ConeyLiu

Copy link
Copy Markdown
ContributorAuthor

thanks @cloud-fan, just move those files to java directory.
Hi @srowen, Offset.java already under java directory.

@SparkQA

Copy link
Copy Markdown

Test build #104038 has finished for PR 24222 at commit d893909.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen

Copy link
Copy Markdown
Member

I'm looking at sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/Offset.java -- seems like that should move too?

@ConeyLiu

Copy link
Copy Markdown
ContributorAuthor

thanks @srowen for point out it. I have moved it too.

@attilapirosattilapiros left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

I have not found any other '*.java" file under scala directories which can be moved.
I used the following search:

$ find . -name "scala" -exec find \{} -name "*.java" \; | grep -v package-info
./mllib/src/main/scala/org/apache/spark/mllib/JavaPackage.java
./sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/BaseStreamingSink.java
./sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/BaseStreamingSource.java
./sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/Offset.java

JavaPackage.java must be kept in its current place as:

/**
* A dummy class as a workaround to show the package doc of <code>spark.mllib</code> in generated
* Java API docs.
* @see <a href="http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4492654" target="_blank">
* JDK-4492654</a>
*/

@srowensrowen 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.

LG pending tests

@SparkQA

Copy link
Copy Markdown

Test build #104047 has finished for PR 24222 at commit 0b69d45.

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

@srowen

Copy link
Copy Markdown
Member

Merged to master

@ConeyLiu

Copy link
Copy Markdown
ContributorAuthor

thanks everyone.

@ConeyLiu
ConeyLiu deleted the move-scala-to-java branch March 29, 2019 01:14
mccheah pushed a commit to palantir/spark that referenced this pull request Jun 6, 2019
## What changes were proposed in this pull request?
move
```scala
org.apache.spark.sql.execution.streaming.BaseStreamingSource
org.apache.spark.sql.execution.streaming.BaseStreamingSink
```
to java directory
## How was this patch tested?
Existing UT.
Closesapache#24222 from ConeyLiu/move-scala-to-java.
Authored-by: Xianyang Liu <xianyang.liu@intel.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
Sign up for freeto 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.

6 participants

@ConeyLiu@srowen@dongjoon-hyun@SparkQA@cloud-fan@attilapiros