Skip to content

Conversation

@WeichenXu123
Copy link
Contributor

What changes were proposed in this pull request?

Revert SPARK-14681 to avoid API breaking change. PR [SPARK-14681] will break mleap.

How was this patch tested?

N/A

@cloud-fan
Copy link
Contributor

Since this changes public API, shall we also revert it from master?

@SparkQA
Copy link

SparkQA commented Sep 20, 2018

Test build #96352 has finished for PR 22492 at commit d976a7e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • sealed abstract class Node extends Serializable

@WeichenXu123
Copy link
Contributor Author

@mengxr Should this be put into master ?

@mengxr
Copy link
Contributor

mengxr commented Sep 21, 2018

We can keep it in master if the next release is 3.0.

asfgit pushed a commit that referenced this pull request Sep 21, 2018
## What changes were proposed in this pull request?

Revert SPARK-14681 to avoid API breaking change. PR [SPARK-14681] will break mleap.

## How was this patch tested?

N/A

Closes #22492 from WeichenXu123/revert_tree_change.

Authored-by: WeichenXu <[email protected]>
Signed-off-by: Xiangrui Meng <[email protected]>
@mengxr
Copy link
Contributor

mengxr commented Sep 21, 2018

LGTM. Merged into branch-2.4. @WeichenXu123 Next time please create dedicated JIRAs for each QA task PR. Thanks!

@mengxr
Copy link
Contributor

mengxr commented Sep 21, 2018

@WeichenXu123 Please close this PR manually. Thanks!

@cloud-fan
Copy link
Contributor

The next version is very likely to be 2.5.0...

mengxr pushed a commit to mengxr/spark that referenced this pull request Oct 2, 2018
## What changes were proposed in this pull request?

Revert SPARK-14681 to avoid API breaking change. PR [SPARK-14681] will break mleap.

## How was this patch tested?

N/A

Closes apache#22492 from WeichenXu123/revert_tree_change.

Authored-by: WeichenXu <[email protected]>
Signed-off-by: Xiangrui Meng <[email protected]>
@mengxr
Copy link
Contributor

mengxr commented Oct 2, 2018

@WeichenXu123 @cloud-fan I made #22618 to revert the change in master.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Oct 2, 2018

May I ask a question, @mengxr ? The master becomes 3.0 today by @gatorsmile 's PR and I saw the following your comment here.

We can keep it in master if the next release is 3.0.

Is there any new reason to change the decision?

@mengxr
Copy link
Contributor

mengxr commented Oct 2, 2018

@cloud-fan said above the next version is very likely to be 2.5.0 instead of 3.0. Well the next version number is not fully discussed yet. For that reason, I think we should revert the changes in master as well. After that we should check if the feature itself can be added without introducing breaking changes.

@dongjoon-hyun
Copy link
Member

Thank you! Although master becomes 3.0.0 already and seems to start to target Mar 2019, I agree with you on reverting for rechecking, @mengxr . Spark 3.0.0 has better have the minimal number of incompatible APIs.

asfgit pushed a commit that referenced this pull request Oct 7, 2018
## What changes were proposed in this pull request?

This is the same as #22492 but for master branch. Revert SPARK-14681 to avoid API breaking changes.

cc: WeichenXu123

## How was this patch tested?

Existing unit tests.

Closes #22618 from mengxr/SPARK-25321.master.

Authored-by: WeichenXu <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@WeichenXu123 WeichenXu123 deleted the revert_tree_change branch October 8, 2018 02:20
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?

This is the same as apache#22492 but for master branch. Revert SPARK-14681 to avoid API breaking changes.

cc: WeichenXu123

## How was this patch tested?

Existing unit tests.

Closes apache#22618 from mengxr/SPARK-25321.master.

Authored-by: WeichenXu <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
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