-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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-11380. Fix hadoop-yarn-api module Java Doc Errors. #5152
Conversation
@ayushtkn Can you help review this PR? Thank you very much! |
💔 -1 overall
This message was automatically generated. |
Thank you very much for reviewing the code! I will fix it. |
💔 -1 overall
This message was automatically generated. |
The following error is remaining: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5152/2/artifact/out/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt
Would you fix it? |
Thank you very much for your help reviewing the code, I will fix this problem. |
💔 -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.
this isn't required, it isn't a javadoc AFAIK, is it also giving failures
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 is a checkstyle problem, not a java doc problem. I see that package-info.java contains less content, so I will fix it. I will roll back this part of the code.
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 rolled back the code, but found that it cannot be compiled by java-doc, we still need to add * to line 1 of package-info.java.
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.
Ok
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.
There are bunch of package-info.java file which doesn't do this. like:
https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/package-info.java
https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/package-info.java
Why it is a problem here only
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 for your help reviewing the code!
These package-info.java will have this problem in JDK11, we can see the following test report
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5151/8/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt
I found that only when there is a new javadoc warning or error, the Java doc error reminder will be triggered. From my personal point of view, we should fix it, so that this module can be completely compiled through JDK11.
💔 -1 overall
This message was automatically generated. |
💔 -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.
LGTM
@ayushtkn Thank you very much for your help in reviewing the code! |
JIRA: YARN-11380. Fix hadoop-yarn-api module Java Doc Errors.
When finishing YARN-11373, ps(#5146) has java-doc compilation errors, I will fix all java-doc compilation errors of hadoop-yarn-api module. I will pay attention to the java doc compilation of jdk8 & jdk11 at the same time.