-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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.
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
[SPARK-51153][BUILD] Remove unused javax.activation:activation
dependency
#49876
Conversation
It comes from https://github.com/apache/spark/pull/22081/files#r209707448, but after #49854 , it seems that nowhere depends on this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this looks unused now.
A minor request: can you please also remove mention from LICENSE-binary?
https://github.com/apache/spark/blob/master/LICENSE-binary#L504
@@ -501,7 +501,6 @@ core/src/main/resources/org/apache/spark/ui/static/d3.min.js | |||
|
|||
Common Development and Distribution License (CDDL) 1.0 | |||
------------------------------------------------------ | |||
javax.activation:activation http://www.oracle.com/technetwork/java/javase/tech/index-jsp-138795.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cnauroth Here it is. Maybe you didn't notice the change here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I do see your change there, but it seems there is one more mention in LICENSE-binary at the root of the repo that could be removed:
https://github.com/apache/spark/blob/master/LICENSE-binary#L504
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the line 504 of LICENSE-binary
, isn't it, @cnauroth ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry all, misread this as coming from a different file. Thanks for the patch, @wayneguow .
javax.activation:activation
dependencejavax.activation:activation
dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM if CI passed.
|
…ndency ### What changes were proposed in this pull request? This PR aims to remove unused `javax.activation:activation` dependence. ### Why are the changes needed? Reduce useless dependencies of Spark. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #49876 from wayneguow/drop_activation. Authored-by: Wei Guo <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 6416297) Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
This PR aims to remove unused
javax.activation:activation
dependence.Why are the changes needed?
Reduce useless dependencies of Spark.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass GA.
Was this patch authored or co-authored using generative AI tooling?
No.