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

Issue 1275 broken javadoc #1753

Merged
merged 10 commits into from Oct 11, 2023

Conversation

lukaszspyra
Copy link
Contributor

Ref. #1275
Partly fixes failures of javadoc:javadoc goal. Changes to api and core modules:

  1. added captions for tables
  2. added descriptions for param names
  3. replaced <h4> headers by <strong> tags (h4 caused 'headings out of sequence' errors)
  4. fixed links by adding fqn
  5. fixed typos in param/javadoc tags names
  6. added missing curly braces
  7. @ escaped with html code &#64; in <pre> tags (it was treated as javadoc tag otherwise)
  8. changed style of some @param tag description according to style guidelines
  9. Removed @return from void methods
  10. the @value - replaced by hardcoded values, as it would render correctly only for compile-time constants

Changes to main pom.xml:
Edited maven javadoc doclint profile, setting to exclude warnings from missing javadoc and html5 semantics only.

@lukaszspyra lukaszspyra mentioned this pull request Sep 2, 2023
@vy vy self-assigned this Oct 5, 2023
@vy vy added the enhancement Additions or updates to features label Oct 5, 2023
@vy
Copy link
Member

vy commented Oct 5, 2023

@lukaszspyra, thanks so much for all these fixes! Much appreciated! 🙇 I have two remarks:

  1. ./mvnw javadoc:javadoc is still failing for -api and -core modules on a few files. Could you also fix them too, please?
  2. What is the rationale behind all,-missing,-html? I mean, if we are gonna suppress errors with all,-missing,-html, how does this contribute to Javadoc is broken #1275?

Links were removed due to following reasons: dependency commons-lang3 not containing docs sources; package sun.reflect.Reflection was deprecated and removed in JDK8
@lukaszspyra
Copy link
Contributor Author

Hi @vy ,
Thanks for the review.

  1. Fixed most remaining errors (removed links which could not be fixed due to withdrawn sun.reflect API, but are meaningful), except 2 links to other modules - could not find a way to make them 'clickable'.
  2. Standard Doclet introduced in JDK8, used for javadoc validation contains many rules, some useful, whilst some annoying. In PR, there is profile created and set to respect all validations excluding:
  • -missing javadoc tags - most of them are either on deprecated API, self-explanatory parameters names or due to usage of brief comments in form of javadoc (gives in total ~150 warnings, majority as @param/@return tags not included). I believe it is reasonable to ignore them.

  • -html strict semantics of HTML5 (all kinds of html tags not supported in HTML5 - e.g. align/summary in table cells, self-closing elements not allowed, empty tags). Generally gives ~100 errors, complaining a lot about tables layout options not recognized in javadoc. You can remove this exclusion and assess yourself, as was not sure about recommended approach.

  1. javadoc generation attached to maven-verify phase as required in issue description

pom.xml Show resolved Hide resolved
pom.xml Show resolved Hide resolved
Copy link
Member

@vy vy left a comment

Choose a reason for hiding this comment

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

Thanks so much! Could you implement the following changes, please?

  1. Target to branch release/2.21.0
  2. Add src/changelog/2.21.0/1275_fix-javadoc.xml (type=fixed, authors are )
  3. Remove javadoc goal attached to the verify phase
  4. Remove <failOnError>false in generate-site-javadoc execution
  5. Fix ./mvnw site failures (I will share some of them below)
/home/vy/Projects/logging-log4j2.git/issue-1275-broken-javadoc/log4j-api/src/main/java/org/apache/logging/log4j/message/ThreadDumpMessage.java:169: error: unexpected content
     * @see /log4j-core/src/main/resources/META-INF/services/org.apache.logging.log4j.message.ThreadDumpMessage$ThreadInfoFactory
/home/vy/Projects/logging-log4j2.git/issue-1275-broken-javadoc/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppender.java:36: error: reference not found
 * {@link org.apache.logging.log4j.core.appender.db.jdbc JDBC}, {@link org.apache.logging.log4j.core.appender.db.jpa
                                                                       ^
/home/vy/Projects/logging-log4j2.git/issue-1275-broken-javadoc/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThreadContextDataInjector.java:105: error: @param name not found
         * @param contextData a {@code StringMap} instance from the log event

@vy vy changed the base branch from 2.x to release/2.21.0 October 11, 2023 10:48
@vy
Copy link
Member

vy commented Oct 11, 2023

@lukaszspyra, you know what, nevermind. I will implement the requested changes myself. We are on a roll with 2.21.0 release and I want to get your changes in before we seal it.

@vy vy merged commit 1350c8d into apache:release/2.21.0 Oct 11, 2023
vy added a commit that referenced this pull request Oct 11, 2023
@vy vy added this to the 2.21.0 milestone Oct 11, 2023
@vy vy added documentation Pull requests or issues that affect documentation api Affects the public API bug Incorrect, unexpected, or unintended behavior of existing code labels Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the public API bug Incorrect, unexpected, or unintended behavior of existing code documentation Pull requests or issues that affect documentation enhancement Additions or updates to features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants