-
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
HADOOP-18576. Lack of a JavaDoc comment before annotation causes Java 11 JavaDoc failure #5344
Conversation
Add JavaDoc comments to package-info.java to avoid errors resulting from the use of Hadoop annotations.
💔 -1 overall
This message was automatically generated. |
Dupes #5226 / HADOOP-18576 |
it does fix 29 of the complaints though, which #5226 didn't. is it just the lack of a doc comment which broke things then, not the @interfaceAudience tag? if so, yes, let's merge -but use the original JIRA ID |
This is different because it addresses the JavaDoc errors. I'd be happy with any fix that unblocks other pull requests. |
I also don't think it's a tag issue, we can modify doc comment to solve this issue. @snmvaughan Thank you for your contribution, but can you modify the title of the pr, as suggested by @steveloughran |
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 am not sure how it was tested actually by folks, atleast the Jenkins just checked hadoop-common, not the entire project.
I tried that and it fails further at yarn
[ERROR] /home/ayushsaxena/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/recovery/package-info.java:19: error: unknown tag: InterfaceAudience.Private
[ERROR] @InterfaceAudience.Private
[ERROR] ^
[ERROR] /home/ayushsaxena/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/recovery/package-info.java:20: error: unknown tag: InterfaceStability.Unstable
[ERROR] @InterfaceStability.Unstable
[ERROR] ^
[ERROR]
[ERROR] Command line was: /usr/lib/jvm/java-11-openjdk-amd64/bin/javadoc @options @packages
[ERROR]
[ERROR] Refer to the generated Javadoc files in '/home/ayushsaxena/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/target/site/apidocs' dir.
[ERROR]
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn <args> -rf :hadoop-yarn-server-resourcemanager
Now about the current fix, the JDK issue leads to issues for package-info.java files and can be workaround if we add Javadocs above these Interface Tags, this PR added them and made them happy, so hadoop-common passed.
- Do we want to add these Javadocs for all package-info files?
- We don't need them, Still?
- Once Java issue is sorted will we revert these?
- If we take this approach we are gonna do it for all modules with these errors? That looks like a brute-force approach to me.
I don't have objections around any approach, I am ok with whichever approach folks feel good , but I would have personally gone with this if the previous proposed fix had issues, just because it looked more clean to me and bothered less code and can be easily reverted in future....
Over to you folks, juzz my thoughts :-)
I looked at the class JavaDoc comments and tried to come up with something useful for the package JavaDoc. I think a brute force approach (which does work) would be to add empty comments above the annotations to address the errors.
|
I tested this fix in the development environment you get from running
|
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, merging. And least now we have a way to fix javadocs everywhere
ok, in trunk. @snmvaughan can you cherrypick the pr into branch-3.3 and resubmit it to see how yetus reacts there. |
…pache#5344) Add JavaDoc comments to package-info.java to avoid errors resulting from the use of Hadoop annotations. Contributed by Steve Vaughan Jr
I've submitted PR #5362 |
This is one Jenkins report from yesterday: With the same error I posted above. Just posting as FYI., @steveloughran I hope that was intentional while merging this, like we just want to fix this much? The branch had this commit as well(4th in the list): |
the goal is to shut javadoc up... |
looks like something got left out; will do a quick followup
|
I was focussing on those modules that were holding up other PRs I've submitted. The same technique could be applied to all modules. |
…pache#5344) Add JavaDoc comments to package-info.java to avoid errors resulting from the use of Hadoop annotations. Contributed by Steve Vaughan Jr
…pache#5344) Add JavaDoc comments to package-info.java to avoid errors resulting from the use of Hadoop annotations. Contributed by Steve Vaughan Jr
…5344) Add JavaDoc comments to package-info.java to avoid errors resulting from the use of Hadoop annotations. Contributed by Steve Vaughan Jr
Description of PR
Add JavaDoc comments to package-info.java to avoid errors resulting from the use of Hadoop annotations.
How was this patch tested?
Running JavaDoc using Java 11 in an Hadoop development environment docker image.
These changes eliminate the JavaDoc errors.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?