-
Couldn't load subscription status.
- Fork 9.1k
YARN-11036. Do not inherit from TestRMWebServicesCapacitySched #3895
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
Change-Id: I48e5b1e020d5cc6fb5ae4a7edc38e9212f4f069b
|
🎊 +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.
@tomicooler thanks for the patch, looks good to me.
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.
Thanks for the patch @tomicooler. The change looks good to me, I had only one nit, aside from it, LGTM.
|
|
||
| public static WebAppDescriptor createWebAppDescriptor() { | ||
| return new WebAppDescriptor.Builder( | ||
| "org.apache.hadoop.yarn.server.resourcemanager.webapp") |
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 looks like a volatile constant here. We could use package name of the class here.
Change-Id: I73f0396b0230c7cad6fad33b924c601a9303ab54
|
🎊 +1 overall
This message was automatically generated. |
|
Thank you for the change @tomicooler. Thanks for the review @brumi1024. |
Change-Id: I48e5b1e020d5cc6fb5ae4a7edc38e9212f4f069b
Description of PR
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?