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.

Sign up for GitHub

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

Jump to bottom

apidoc error/warning fix (#2) #5561

Merged
merged 2 commits into from
Mar 3, 2023
Merged

apidoc error/warning fix (#2) #5561

merged 2 commits into from
Mar 3, 2023

Conversation

ebarboni
Copy link
Contributor

This is a second split of the big PR #4450.
using the following to have ant javadoc -Dmetabuild.branch=master -Dapidocfullcheck=true.

@ebarboni ebarboni requested a review from mbien February 24, 2023 17:01
@mbien mbien added the JavaDoc [ci] enable java/javadoc tests and build-javadoc target label Feb 24, 2023
@apache apache locked and limited conversation to collaborators Feb 24, 2023
@apache apache unlocked this conversation Feb 24, 2023
@mbien
Copy link
Member

mbien commented Feb 24, 2023

restarted build with javadoc label, otherwise it won't build the javadoc

mbien
mbien reviewed Feb 24, 2023
View reviewed changes
Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

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

only took a quick look though it so far

harness/nbjunit/src/org/netbeans/junit/ParametricTestSuite.java
*/
protected abstract Object[] getParameters();

/**
* Factory method returns a new instance of a testcases.
* Overrides the basic method so that it's needless any more.
* @return test case
Copy link
Member

Choose a reason for hiding this comment

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

i gotta say I hate this kind of doc :)

This isn't adding any information it is just auto generated noise. Is there no way to turn off the warnings for things like this? This doesn't actually make the doc any better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how to be more specific for the warning, the return are not critical and I hate writing that too. But as some other warning prevents generation of javadoc I prefer having this noise than having half of apidoc.

Copy link
Member

Choose a reason for hiding this comment

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

missing doc warnings should be harmless as far as I know. They shouldn't cause doc to be not generated at all - at least I never saw that happening before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you have some that are armless like return, some generate plain text instead of links on non existing page (for wrong @link / @see) some. and some break the build in later jdk.
but I can close my eyes on return :p

Copy link
Member

Choose a reason for hiding this comment

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

And for everything else there's doclint:none 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

doclint:html,reference,syntax would be perfect I guess for the needs of having a apidoc in good shape.

Copy link
Contributor Author

@ebarboni ebarboni Feb 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I put an additionnal commit to set doclint to verify only what html/reference/syntax. Thanks for the tip, I do prefer focus on links/href/html as it break website :p. Bad text for a parameter/return can be fixed later if people have concern on that.

I had to relax the failonwarning because javadoc on jdk18 create warning on depreciate with removal of some classes.

@ebarboni ebarboni force-pushed the apidocsanitize2 branch 2 times, most recently from 04c99d0 to 00108e0 Compare February 27, 2023 13:59
pepness
pepness reviewed Mar 1, 2023
View reviewed changes
pepness
pepness reviewed Mar 1, 2023
View reviewed changes
java/classfile/arch.xml Outdated
@@ -182,7 +182,7 @@
-->
<answer id="compat-standards">
<p>
The module depends upon the <a href="http://java.sun.com/docs/books/vmspec/2nd-edition/html/VMSpecTOC.doc.html">
The module depends upon the <a href="https://docs.oracle.com/javase/specs/jvms/se6/html/VMSpecTOC.doc.html">
Copy link
Member

Choose a reason for hiding this comment

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

Same url as the previous comment.

pepness
pepness reviewed Mar 1, 2023
View reviewed changes
pepness
pepness reviewed Mar 1, 2023
View reviewed changes
java/classfile/src/org/netbeans/modules/classfile/ExceptionTableEntry.java Outdated
@@ -79,6 +81,8 @@ public final int getStartPC() {
* Returns the ending offset into the method's bytecodes of this
* exception handler, or the length of the bytecode array if the
* handler supports the method's last bytecodes (JVM 4.7.3).
* @return ending offset of the exception handlong code into
Copy link
Member

Choose a reason for hiding this comment

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

same typo. handler/handling

pepness
pepness reviewed Mar 1, 2023
View reviewed changes
java/classfile/src/org/netbeans/modules/classfile/package.html Outdated
@@ -73,7 +73,7 @@ <h2>Examples</h2>
<h2>Related Documentation</h2>

<ul>
<li><A HREF="http://java.sun.com/docs/books/jls/index.html">Java Virtual Machine Specification, Second Edition</a>
<li><A HREF="hhttps://docs.oracle.com/javase/specs/jvms/se6/html/VMSpecTOC.doc.html">Java Virtual Machine Specification, Second Edition</a>
Copy link
Member

Choose a reason for hiding this comment

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

There is an extra h in http. Consider using https://docs.oracle.com/javase/specs/jvms/se8/html/index.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well seems the scanner for wrong url didn't issue an error. Will not for later check

pepness
pepness reviewed Mar 1, 2023
View reviewed changes
Copy link
Member

@pepness pepness left a comment

Choose a reason for hiding this comment

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

Great work.

@ebarboni ebarboni force-pushed the apidocsanitize2 branch 2 times, most recently from 1025dc6 to de388cb Compare March 2, 2023 10:04
@mbien mbien added this to the NB18 milestone Mar 2, 2023
mbien
mbien approved these changes Mar 2, 2023
View reviewed changes
Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

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

LGTM

@ebarboni
Copy link
Contributor Author

ebarboni commented Mar 3, 2023

ok I merge and go for another round :D

@ebarboni ebarboni merged commit d090d70 into apache:master Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neilcsmith-net neilcsmith-net neilcsmith-net left review comments

@pepness pepness pepness left review comments

@mbien mbien mbien approved these changes

Assignees
No one assigned
Labels
Code cleanup JavaDoc [ci] enable java/javadoc tests and build-javadoc target
Projects
None yet
Milestone
NB18
Development

Successfully merging this pull request may close these issues.
4 participants
@ebarboni @mbien @neilcsmith-net @pepness