Skip to content

[MINOR] Move java file to java directory #24222

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
Closed
wants to merge 3 commits into from
Closed

[MINOR] Move java file to java directory #24222

wants to merge 3 commits into from

Conversation

ConeyLiu
Copy link
Contributor

@ConeyLiu ConeyLiu commented Mar 26, 2019 •
edited
Loading

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
Contributor Author

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

@srowen
Copy link
Member

srowen commented Mar 26, 2019

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

@srowen
Copy link
Member

srowen commented Mar 26, 2019

Offset.java can be moved too?

@dongjoon-hyun
Copy link
Member

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

@dongjoon-hyun
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Mar 26, 2019

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.

@felixcheung felixcheung changed the title [MINOR] Move java file to java foler [MINOR] Move java file to java directory Mar 27, 2019
@ConeyLiu
Copy link
Contributor Author

ConeyLiu commented Mar 27, 2019 •
edited
Loading

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
Contributor

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

@ConeyLiu
Copy link
Contributor Author

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

@SparkQA
Copy link

SparkQA commented Mar 28, 2019

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
Member

srowen commented Mar 28, 2019

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
Contributor Author

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

attilapiros
attilapiros approved these changes Mar 28, 2019
View reviewed changes
Copy link
Contributor

@attilapiros attilapiros left a comment

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>
 */

srowen
srowen approved these changes Mar 28, 2019
View reviewed changes
Copy link
Member

@srowen srowen left a comment

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

SparkQA commented Mar 28, 2019

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
Member

srowen commented Mar 28, 2019

Merged to master

@srowen srowen closed this in 50cded5 Mar 28, 2019
@ConeyLiu
Copy link
Contributor Author

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
@ConeyLiu @mccheah
[MINOR] Move java file to java directory
d8f503e 
## 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.

Closes apache#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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srowen srowen srowen approved these changes

@attilapiros attilapiros attilapiros approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ConeyLiu @srowen @dongjoon-hyun @SparkQA @cloud-fan @attilapiros