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

Enabling missingJavadoc validation in CI with gradle check #721

Merged
merged 1 commit into from
May 25, 2021

Conversation

setiah
Copy link
Contributor

@setiah setiah commented May 19, 2021

This PR enables missingJavadoc validation as part of gradle check.

It excludes modules currently failing from missingJavadoc check from validation so tht they don't gradle 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. This will prevent any new module that is without javadocs from getting added as the gradle check would fail. It will also prevent any new changes without javadocs on existing modules that are not part of exclusion list (e.g. :client:rest, thanks to #685 )

Signed-off-by: Himanshu Setia setiah@amazon.com

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

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 <setiah@amazon.com>
@setiah setiah changed the title Enabling missingJavadoc validation in gradle check Enabling missingJavadoc validation in CI with gradle check May 19, 2021
@setiah setiah mentioned this pull request May 19, 2021
86 tasks
@setiah setiah requested review from adnapibar and nknize May 19, 2021 00:40
@setiah
Copy link
Contributor Author

setiah commented May 19, 2021

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 7996a8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 7996a8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 7996a8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 7996a8e
Log 187

Reports 187

project(":test:fixtures:s3-fixture"),
project(":test:framework"),
project(":test:logger-usage")
]) {
Copy link
Member

@dblock dblock May 20, 2021

Choose a reason for hiding this comment

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

This is basically everything, especially with :server. Feels like we should machine-generate JavaDoc first, or divide and conquer manually adding it, or something like that before enabling this. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing @dblock. I agree we should add more javadocs, and divide and conquer seems to be the right approach, given the extent. However to your point on publishing, we do have machine generated javadocs today on opensearch website. Enabling this check will not break the javadoc task (used for publishing these docs on website), instead it would ensure that the modules that have been fixed (like client:rest #685) do not break again.

Copy link
Member

@dblock dblock May 21, 2021

Choose a reason for hiding this comment

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

Open an issue for missing JavaDoc when merging and link to this conversation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open an issue for missing JavaDoc when merging and link to this conversation?

Already there :) #221

@setiah setiah merged commit 971adc6 into opensearch-project:main May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants