Skip to content
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

Correct Javadoc warnings, prevent future ones #3898

Merged
merged 17 commits into from
Jun 2, 2023

Conversation

niloc132
Copy link
Member

Addresses several classes of issues, some in more than one way:

  • Ensure all javadoc runs use UTF-8 encoding, and remove non-ASCII chars which are intended to be interpreted as code
  • HTML-encode characters that must be encoded in javadoc, and correct wrongly-encoded chars.
  • Don't reference JVM internals that can't be linked, instead refer to them as @code.
  • Fixed mismatched html tags, broken attrs.
  • Corrected/removed classes/members that don't exist or can't be referenced in that way.

Fixes #1513

@niloc132 niloc132 added documentation Improvements or additions to documentation build NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels May 31, 2023
@niloc132 niloc132 requested a review from devinrsmith May 31, 2023 21:09
@niloc132
Copy link
Member Author

This is left as a draft for the moment to get some feedback from a few specific authors/owners. Only combined-javadoc will fail if there are warnings, allowing potentially "invalid" javadoc references that can be resolved by producing javadoc all at once.


options.encoding = 'UTF-8'

options.links = ['https://docs.oracle.com/en/java/javase/11/docs/api/']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 11?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing b/c that's the language level we target.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just centralized the javadoc configuration to one place, so everything is built the same way, rather than 3-ish different setups.

options.encoding = 'UTF-8'

options.links = ['https://docs.oracle.com/en/java/javase/11/docs/api/']
options.addBooleanOption('Xdoclint:none', true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping -quiet was intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it did nothing, it was a workaround for a gradle javadoc limitation. Boolean is probably more accurate or at least less confusing, so no one thinks that -quiet was intended to be added as a real flag.

devinrsmith
devinrsmith previously approved these changes Jun 1, 2023
Copy link
Member

@devinrsmith devinrsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildSrc changes look good. I did go fast through the actual javadoc changes. I call this a big win, even if we feel like there are places we might want to follow-up on to make further improvements. I would not hold up this PR for javadoc content-type changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the only file that stood out as having some sort of new code - I'm assuming you re-ran the generation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen was updated, else the build would fail when the assert check ran.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question.

@niloc132 niloc132 enabled auto-merge (squash) June 2, 2023 19:37
@niloc132 niloc132 merged commit 4c5fc89 into deephaven:main Jun 2, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build documentation Improvements or additions to documentation NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove non-LTS JDK 15 as part of javadocs process
4 participants