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

MissingJavaDoc checker to support * for ignoring group of files #7264

Closed
saratvemulapalli opened this issue Apr 20, 2023 · 6 comments · Fixed by #7604
Closed

MissingJavaDoc checker to support * for ignoring group of files #7264

saratvemulapalli opened this issue Apr 20, 2023 · 6 comments · Fixed by #7604
Labels
enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers untriaged

Comments

@saratvemulapalli
Copy link
Member

Is your feature request related to a problem? Please describe.
MissingJavaDoc gradle task doesnt support ignoring group of files.
For example: Generated code dont follow our standards and have to be ignored.
Coming from PR: #6960 (comment)

Describe the solution you'd like
Add support to ignore multiple files with *

Eg:
Before:

javadocMissingIgnore = [
    "org.opensearch.extensions.proto.ExtensionRequestProto",
    "org.opensearch.extensions.proto.ExtensionRequestProto.ExtensionRequestOrBuilder",
    "org.opensearch.extensions.proto"
  ]

After:

javadocMissingIgnore = [
    "org.opensearch.extensions.proto.*"
  ]
@saratvemulapalli saratvemulapalli added enhancement Enhancement or improvement to existing feature or request untriaged good first issue Good for newcomers labels Apr 20, 2023
@dbwiddis
Copy link
Member

OpenSearch might want to consider using Checkstyle for javadocs as we do on SDK. I know Checkstyle was removed for style, but it's got an extremely powerful feature set for configuring javadoc checks and if it's limited to those, I think it could replace our existing missing javadoc check, which is an unmaintained, copied-from-Lucene/Solr hack that I don't think anyone should spend time updating.

See also #4541

@austintlee
Copy link
Contributor

I had a quick look and this looks like a fairly straightforward thing to support (a few lines of code). But is it "safe" to support this? It seems to me that perhaps this was by design. Supporting '*' could lead to misuse/abuse.

@dblock
Copy link
Member

dblock commented May 1, 2023

+1 to what @austintlee says, but also maybe it's NBD and a fine workaround to mark generated code as such

Another option is to generate code that complies with our checkstyle.

@austintlee
Copy link
Contributor

@saratvemulapalli I created a PR to suggest a possible way to make this work using an allowlist. One question I have is around where that allowlist should be placed. Please, take a look. Thanks!

@austintlee
Copy link
Contributor

If we are OK with this approach in general, I can also add some unit tests for MissingDoclet. At the moment, this whole package (org.opensearch.missingdoclet) has no unit tests.

@austintlee
Copy link
Contributor

I am proceeding with an approach that allows only generated classes annotated with '@javax.annotation.Generated' to be ignored.

Of course, all other exceptions will still need to be added to build.gradle manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers untriaged
Projects
None yet
4 participants