-
Notifications
You must be signed in to change notification settings - Fork 80
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
Conversation
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. |
DHProcess/src/main/java/io/deephaven/process/SystemCpuOshi.java
Outdated
Show resolved
Hide resolved
Util/src/main/java/io/deephaven/util/audit/AuditEventLoggerBasic.java
Outdated
Show resolved
Hide resolved
ModelFarm/src/main/java/io/deephaven/modelfarm/ModelFarmOnDemand.java
Outdated
Show resolved
Hide resolved
ModelFarm/src/main/java/io/deephaven/modelfarm/RDMModelFarm.java
Outdated
Show resolved
Hide resolved
|
||
options.encoding = 'UTF-8' | ||
|
||
options.links = ['https://docs.oracle.com/en/java/javase/11/docs/api/'] |
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.
Why 11?
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'm guessing b/c that's the language level we target.
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.
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) |
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.
Dropping -quiet
was intentional?
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.
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.
engine/table/src/main/java/io/deephaven/engine/table/impl/sources/FillUnordered.java
Show resolved
Hide resolved
...ble/src/main/java/io/deephaven/engine/table/impl/updateby/emstd/BigDecimalEmStdOperator.java
Show resolved
Hide resolved
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.
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.
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 was the only file that stood out as having some sort of new code - I'm assuming you re-ran the generation?
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.
Same question.
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.
Codegen was updated, else the build would fail when the assert check ran.
Configuration/src/main/java/io/deephaven/configuration/PropertyFile.java
Show resolved
Hide resolved
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.
Same question.
Addresses several classes of issues, some in more than one way:
@code
.Fixes #1513