-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HADOOP-18203. Backport HADOOP-15033 to branch-2.10. #4174
base: branch-2.10
Are you sure you want to change the base?
HADOOP-18203. Backport HADOOP-15033 to branch-2.10. #4174
Conversation
…animal-sniffer-maven-plugin to compile with Java 9. (cherry picked from commit d2d8f4a)
💔 -1 overall
This message was automatically generated. |
Contributed by Dmitry Chuyko, (cherry picked from commit 2e0a451)
2b468fa
to
2f75bb7
Compare
💔 -1 overall
This message was automatically generated. |
private static final int JAVA_SPEC_VER = Math.max(8, Integer.parseInt( | ||
System.getProperty("java.specification.version").split("\\.")[0])); |
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.
JAVA_SPEC_VER is set to 8 even on Java 7. Can we cover Java 7 here? If not, we should have a comment for justification at least.
public void testIsJavaVersionAtLeast() { | ||
assertTrue(Shell.isJavaVersionAtLeast(8)); | ||
} |
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 test succeeds on Java 7 since the Shell.isJavaVersionAtLeast(8)
returns true even on Java 7. We should have a comment for justification at least.
Description of PR
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?