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

[Meta] Add javadocs #221

Open
2 of 86 tasks
setiah opened this issue Mar 5, 2021 · 6 comments
Open
2 of 86 tasks

[Meta] Add javadocs #221

setiah opened this issue Mar 5, 2021 · 6 comments
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed Meta Meta issue, not directly linked to a PR

Comments

@setiah
Copy link
Contributor

setiah commented Mar 5, 2021

Description
This is a meta issue to track javadoc progress for all modules that are currently missing javadocs. Once javadocs are added, we can enable missingJavadoc task as part of gradle checks, to enable strict javadoc validation.

How to reproduce
<Needs update once #721 is merged>

  1. Clone the repo (default main branch) and switch to remote javadoc branch (Note: javadoc branch is deprecated now)
  2. Run ./gradlew missingJavadoc
  3. Check the javadoc-output.txt file for failures due to missing javadocs

Logs

* Where:
Script '/Users/setiah/projects/odfe/search/gradle/missing-javadoc.gradle' line: 260

* What went wrong:
Execution failed for task ':build-tools:missingJavadoc'.
> Javadoc generation failed for :build-tools,
    Options file at: /Users/setiah/projects/odfe/search/buildSrc/build/tmp/missingJavadoc/javadoc-options.txt
    Command output at: /Users/setiah/projects/odfe/search/buildSrc/build/tmp/missingJavadoc/javadoc-output.txt <--------

Subtasks
Currently most modules are failing javadoc validations due to missing javadocs.

  • :benchmarks
  • :build-tools
  • :build-tools:reaper
  • :client:benchmark
  • :client:client-benchmark-noop-api-plugin
  • :client:rest
  • :client:rest-high-level
  • :client:sniffer
  • :client:test
  • :client:transport
  • :distribution:tools:java-version-checker
  • :distribution:tools:keystore-cli
  • :distribution:tools:launchers
  • :distribution:tools:plugin-cli
  • :doc-tools
  • :example-plugins:custom-settings
  • :example-plugins:custom-significance-heuristic
  • :example-plugins:custom-suggester
  • :example-plugins:painless-whitelist
  • :example-plugins:rescore
  • :example-plugins:rest-handler
  • :example-plugins:script-expert-scoring
  • :libs:opensearch-cli
  • :libs:opensearch-core
  • :libs:opensearch-dissect
  • :libs:opensearch-geo
  • :libs:opensearch-grok
  • :libs:opensearch-nio
  • :libs:opensearch-plugin-classloader
  • :libs:opensearch-secure-sm
  • :libs:opensearch-ssl-config
  • :libs:opensearch-x-content
  • :modules:aggs-matrix-stats
  • :modules:analysis-common
  • :modules:geo
  • :modules:ingest-common
  • :modules:ingest-geoip
  • :modules:ingest-user-agent
  • :modules:lang-expression
  • :modules:lang-mustache
  • :modules:lang-painless
  • :modules:lang-painless:spi
  • :modules:mapper-extras
  • :modules:opensearch-dashboards
  • :modules:parent-join
  • :modules:percolator
  • :modules:rank-eval
  • :modules:reindex
  • :modules:repository-url
  • :modules:systemd
  • :modules:transport-netty4
  • :plugins:analysis-icu
  • :plugins:analysis-kuromoji
  • :plugins:analysis-nori
  • :plugins:analysis-phonetic
  • :plugins:analysis-smartcn
  • :plugins:analysis-stempel
  • :plugins:analysis-ukrainian
  • :plugins:discovery-azure-classic
  • :plugins:discovery-ec2
  • :plugins:discovery-ec2:qa:amazon-ec2
  • :plugins:discovery-gce
  • :plugins:discovery-gce:qa:gce
  • :plugins:ingest-attachment
  • :plugins:mapper-annotated-text
  • :plugins:mapper-murmur3
  • :plugins:mapper-size
  • :plugins:repository-azure
  • :plugins:repository-gcs
  • :plugins:repository-hdfs
  • :plugins:repository-s3
  • :plugins:store-smb
  • :plugins:transport-nio
  • :qa:die-with-dignity
  • :qa:os
  • :qa:wildfly
  • :rest-api-spec
  • :server ([Javadoc] Add missing package-info.java files to server #3128, [Javadocs] add to o.o.action.admin #3155, [Javadocs] add to o.o.bootstrap, cli, and client #3163)
  • :test:external-modules:test-delayed-aggs
  • :test:fixtures:azure-fixture
  • :test:fixtures:gcs-fixture
  • :test:fixtures:hdfs-fixture
  • :test:fixtures:old-elasticsearch
  • :test:fixtures:s3-fixture
  • :test:framework
  • :test:logger-usage

How to help
Please add javadocs for the module you work on. Validate the module using the missingJavadoc gradle task.

For example, if you are adding javadocs for :client:rest module, you can validate your changes using command ./gradlew :client:rest:missingJavadoc command.

@setiah setiah changed the title Add javadocs [DOCS] Add javadocs Mar 5, 2021
@setiah setiah added documentation Improvements or additions to documentation Meta Meta issue, not directly linked to a PR labels Mar 5, 2021
@setiah setiah closed this as completed Mar 22, 2021
@setiah setiah reopened this Mar 23, 2021
@setiah setiah changed the title [DOCS] Add javadocs Add javadocs Mar 23, 2021
@setiah setiah added the help wanted Extra attention is needed label Mar 23, 2021
setiah pushed a commit to setiah/OpenSearch that referenced this issue May 19, 2021
Excludes modules currently failing from missingJavadoc check.
These modules can be removed from exclusion list once javadocs are added.
Currently, only :client:rest module is enabled for missingJavadoc check.
Issue opensearch-project#221 tracks javadocs for legacy code.

These checks are enabled as part of gradle check. Enabling it as part of
precommit needs more work and will be done later.

Once this PR is merged all incoming PRs will undergo javadoc validations.

Signed-off-by: Himanshu Setia <[email protected]>
@setiah setiah changed the title Add javadocs [Meta] Add javadocs May 19, 2021
setiah added a commit that referenced this issue May 25, 2021
Excludes modules currently failing from missingJavadoc check.
These modules can be removed from exclusion list once javadocs are added.
Currently, only :client:rest module is enabled for missingJavadoc check.
Issue #221 tracks javadocs for legacy code.

These checks are enabled as part of gradle check. Enabling it as part of
precommit needs more work and will be done later.

Once this PR is merged all incoming PRs will undergo javadoc validations.

Signed-off-by: Himanshu Setia <[email protected]>
@anasalkouz
Copy link
Member

Hi @setiah, do you have plan to add missing javadocs for this list of modules? how to track the progress?

@setiah
Copy link
Contributor Author

setiah commented Nov 11, 2021

Hey @anasalkouz adding javadocs for all modules is a behemoth effort which we need to divide and conquer with the help from community. Evaluating other priorities, we need to bring this in our sprint planning and conquer module-by-module and parallelize where possible. For some modules like server, which are huge in themselves, might need to be further split this into sub-tasks. As we add javadocs, we need to remove those modules from the exclusion list for javadoc checks. Again, this is a behemoth task which might need to evaluated against other priorities, but definitely something we should be looking into.

@nknize
Copy link
Collaborator

nknize commented May 2, 2022

Given that we have a number of plugins extending internal class functionality that shouldn't be extended, I am going to start pushing progress toward enabling the missingJavadoc gradle task so we can start enforcing documentation on public classes and methods.

This will also include two new javadoc annotations (@opensearch.internal and @opensearch.experimental) for documenting internal classes that should not be extended in external plugins.

NOTE: THIS SHOULD NOT ACT AS A SUBSTITUTE FOR USING GOOD CLASS/METHOD MODIFIERS (e.g., final, private, protected, and public).

/cc @anasalkouz @kartg @setiah @saratvemulapalli

@minalsha
Copy link
Contributor

@nknize how should we take this forward so that respective teams can pick up and we make progress on this meta issue?

@ker2x
Copy link
Contributor

ker2x commented Oct 12, 2023

Should we "javadocument" testcase or ignore them for now ?

@dblock
Copy link
Member

dblock commented Oct 13, 2023

@ker2x I would skip tests.

ritty27 pushed a commit to ritty27/OpenSearch that referenced this issue May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed Meta Meta issue, not directly linked to a PR
Projects
None yet
Development

No branches or pull requests

6 participants