[FEATURE] Add Java docs and mandate for all new classes

[saratvemulapalli]

Is your feature request related to a problem?

We are missing Java docs for this repository.

What solution would you like?

Lets add support for Java docs and mandate all new classes to have them.
We could run this check as part of the PR check workflow.

[dbwiddis]

+1. We are small enough right now to fix this without too much effort.

[dbwiddis]

When I submitted a PR on the core repo yesterday the PR template included a check-off for Javadocs. I'll add that to this repo; may not be an automated check but it will be something that submitters will see when they submit a PR, and should prompt code reviewers to do so as well.

Also can we be clear on the policy for Javadocs? My thoughts:

• Required for all classes
• Required for all public and protected methods, with the exception of getters and setters where the only javadoc would be "gets the foo"; "sets the foo". Optional for private methods if the usage isn't obvious.
  • First line should be short and end in a period.
  • Separate paragraphs with <p>
  • @param required if any parameters. Explicitly state if null is allowed.
  • @param required for generics, e.g., <T>
  • @return required including what return values represent error conditions. Explicitly state if null is returned.
  • @throws required for runtime exceptions that aren't caught

[dbwiddis]

I should have added to the above list. If a method overrides a method in a superclass, it automatically inherits that javadoc so it isn't required -- but can be added with the inheritdoc annotation if there's anything additional to say that the superclass/interface javadoc doesn't cover.

[dbwiddis]

https://github.com/plume-lib/require-javadoc

[dbwiddis]

After spending way too much time yesterday implementing the way that the main project does this, I'm thinking I'd prefer to use the require-javadoc dependency linked in the previous comment.

Existing method:

• requires duplicating (and then tweaking) the entire doc-tools directory from the OpenSearch site, including two build.gradle scripts and a file originated from Apache Solr, making it a derivative of a derivative. Like the game of telephone, the further away from the source the more likely there are to be unknown errors.
• is not configurable at all and is extremely strict
  • doesn't allow skipping for trivial property accessors (getters and setters) or default, no-arg constructors (requires creating a constructor just to javadoc it!)
• will not be automatically updated with bug fixes or features, etc.
• The underlying javadoc checks (param, return, etc.) come from the javadoc tool itself, this import is only used to verify their presence as javadoc won't warn on a missing one.

In contrast, require-javadoc:

• is published on Maven Central and importing it is a one-liner compared to an entire file structure
• is supported: I submitted a feature request yesterday and there's already a PR on it (that I have reviewed)

@saratvemulapalli and @owaiskazi19 what are your thoughts?

[peternied]

I would rather add dependencies on maintained components, I'd cast my vote with either require-javadoc or checkstyle.

If there are missing features in either tool, this would be a good reason to improve them with a proposal and pull request(s) to unblock adoption.

[saratvemulapalli]

@saratvemulapalli and @owaiskazi19 what are your thoughts?

I would always prefer relying on standard implementation vs something for us to maintain down the line.
I would go with require-javadoc, also take our learnings to opensearch repository.

[owaiskazi19]

@saratvemulapalli and @owaiskazi19 what are your thoughts?

Discussed offline. +1 to @saratvemulapalli

[dbwiddis]

OK, will switch to require-javadoc for this repo, and get some lessons learned and if we like it suggest for the main repo.

FYI, I requested plume-lib/require-javadoc#143 to help minimize trivial documentation.