You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/03/04 08:16:53 UTC
[GitHub] [spark] LuciferYang opened a new pull request #35732: [SPARK-38414][CORE] Remove redundant `@SuppressWarnings `
LuciferYang opened a new pull request #35732:
URL: https://github.com/apache/spark/pull/35732
### What changes were proposed in this pull request?
This pr remove redundant `@SuppressWarnings` in Spark Java code, all case inspected by IDE (IntelliJ)
### Why are the changes needed?
Remove redundant `@SuppressWarnings `
### Does this PR introduce _any_ user-facing change?
NO.
### How was this patch tested?
Pass GA
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #35732: [SPARK-38414][CORE] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35732:
URL: https://github.com/apache/spark/pull/35732#issuecomment-1059976400
Yes, no new warnings. I think this should be related to the JDK version upgrade. I will investigate the reasons and reply later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #35732: [SPARK-38414][CORE][DSTREAM][EXAMPLES][ML][MLLIB][SQL] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #35732:
URL: https://github.com/apache/spark/pull/35732#discussion_r820349472
##########
File path: common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java
##########
@@ -197,7 +197,6 @@ public synchronized void close() throws IOException {
* when Scala wrappers are used, this makes sure that, hopefully, the JNI resources held by
* the iterator will eventually be released.
*/
- @SuppressWarnings("deprecation")
Review comment:
Oh, do warnings pop back up? I thought these were found to not suppress anything? if it still emits a warning without it, we should keep it. For example, yeah, isn't finalize() definitely deprecated as of 11?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #35732: [SPARK-38414][CORE][DSTREAM][EXAMPLES][ML][MLLIB][SQL] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35732:
URL: https://github.com/apache/spark/pull/35732#discussion_r820376925
##########
File path: common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java
##########
@@ -197,7 +197,6 @@ public synchronized void close() throws IOException {
* when Scala wrappers are used, this makes sure that, hopefully, the JNI resources held by
* the iterator will eventually be released.
*/
- @SuppressWarnings("deprecation")
Review comment:
Revert the change related Java 11, and others will not pop new warnings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao closed pull request #35732: [SPARK-38414][CORE][DSTREAM][EXAMPLES][ML][MLLIB][SQL] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
huaxingao closed pull request #35732:
URL: https://github.com/apache/spark/pull/35732
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #35732: [SPARK-38414][CORE][DSTREAM][EXAMPLES][ML][MLLIB][SQL] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #35732:
URL: https://github.com/apache/spark/pull/35732#issuecomment-1061021220
Merged to master. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #35732: [SPARK-38414][CORE][DSTREAM][EXAMPLES][ML][MLLIB][SQL] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35732:
URL: https://github.com/apache/spark/pull/35732#issuecomment-1061022855
thanks all
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #35732: [SPARK-38414][CORE] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #35732:
URL: https://github.com/apache/spark/pull/35732#issuecomment-1059977480
JDK 8 -> 11? we still build vs Java 8 though
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #35732: [SPARK-38414][CORE][DSTREAM][EXAMPLES][ML][MLLIB][SQL] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35732:
URL: https://github.com/apache/spark/pull/35732#discussion_r820347307
##########
File path: common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java
##########
@@ -197,7 +197,6 @@ public synchronized void close() throws IOException {
* when Scala wrappers are used, this makes sure that, hopefully, the JNI resources held by
* the iterator will eventually be released.
*/
- @SuppressWarnings("deprecation")
Review comment:
@srowen I found that you added this `@SuppressWarnings` in SPARK-26090 . It seems that this is to suppress the `deprecation` warning when Java 8 -> 11. So we don't need to delete it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #35732: [SPARK-38414][CORE] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35732:
URL: https://github.com/apache/spark/pull/35732#issuecomment-1059979164
no，6 or 7 -> 8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #35732: [SPARK-38414][CORE] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #35732:
URL: https://github.com/apache/spark/pull/35732#issuecomment-1059970606
So there are no new warnings after removing these suppressions? I wonder why so many are no longer necessary; it is possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #35732: [SPARK-38414][CORE] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #35732:
URL: https://github.com/apache/spark/pull/35732#issuecomment-1059988843
OK are there more of these beyond core, and is this all of the occurrences in core?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #35732: [SPARK-38414][CORE][DSTREAM][EXAMPLES][ML][MLLIB][SQL] Remove redundant `@SuppressWarnings `
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35732:
URL: https://github.com/apache/spark/pull/35732#issuecomment-1060131449
> OK are there more of these beyond core, and is this all of the occurrences in core?
Updated PR title
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org