-
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
YARN-10931.Remove some invalid characters in NMClientAsyncImpl#ContainerState. #3365
Conversation
💔 -1 overall
This message was automatically generated. |
@tangzhankun @zhuqi-lucas , can you help review the code. |
It seems that UT has failed to execute, but from the details, I can't see where the problem is. |
@@ -433,7 +433,7 @@ public void getContainerStatusAsync(ContainerId containerId, NodeId nodeId) { | |||
} | |||
|
|||
protected enum ContainerState { | |||
PREP, FAILED, RUNNING, DONE, | |||
PREP, FAILED, RUNNING, DONE |
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.
out of curiosity, what harm is this comma causing?
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 @ayushtkn for the comment. I found that in most places in Hadoop projects, when enum is defined, the last element usually does not have any symbol, so I think the same style needs to be maintained here. This is my idea.
Welcome to continue to communicate.
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 do agree with @ayushtkn, also this comma doesn't qualify for an invalid character.
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 @szilard-nemeth for the comment and review.
…nerState.
jira:YARN-10931