-
Notifications
You must be signed in to change notification settings - Fork 9.1k
YARN-11263. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-yarn-server-nodemanager Part1. #7390
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
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
ad1aa12
to
1364afe
Compare
💔 -1 overall
This message was automatically generated. |
f3fa30e
to
b7eb2c9
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@cnauroth Could you please help review this PR? Thank you very much! |
🎊 +1 overall
This message was automatically generated. |
public boolean https; | ||
|
||
@Before | ||
public void initHttps(boolean pHttps) { |
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.
Can this be made private
?
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 very much for reviewing the code! I will also pay attention to similar issues in other classes and make improvements.
public boolean https; | ||
|
||
@Before | ||
public void initHttps(boolean pHttps) throws ContainerExecutionException { |
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.
Can this be made private
?
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.
I will fix this issue.
🎊 +1 overall
This message was automatically generated. |
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. Thank you again, @slfan1989 .
@cnauroth Thank you very much for reviewing the code! I have merged it into the trunk branch to start the JUnit5 upgrade for NodeManager Part2. |
…odemanager Part1. (apache#7390) [JDK17] Upgrade JUnit from 4 to 5 in hadoop-yarn-server-nodemanager. Co-authored-by: Chris Nauroth <[email protected]> Reviewed-by: Chris Nauroth <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
Description of PR
JIRA: YARN-11263. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-yarn-server-nodemanager.
How was this patch tested?
Junit & mvn clean test.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?