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

[Discuss] Benchmark class method do not have documentation #724

Closed
jainankitk opened this issue May 19, 2021 · 5 comments
Closed

[Discuss] Benchmark class method do not have documentation #724

jainankitk opened this issue May 19, 2021 · 5 comments
Labels
bug Something isn't working Build Libraries & Interfaces discuss Issues intended to help drive brainstorming and decision making Priority-Low

Comments

@jainankitk
Copy link
Collaborator

Describe the bug
Currently benchmark class methods do not have documentation

Expected behavior
Wondering what is our take on that.

  • At one hand benchmarks are like tests and should be fine for test methods to not have documentation?
  • At the other hand, it is good to have documentation for any piece of well written code. In this case, should we enforce by adding javadoc check as part of gradle precommit?
@jainankitk jainankitk added Beta bug Something isn't working untriaged labels May 19, 2021
@jainankitk jainankitk changed the title Benchmark class method do not have documentation [Discuss] Benchmark class method do not have documentation May 19, 2021
@CEHENKLE CEHENKLE added discuss Issues intended to help drive brainstorming and decision making and removed untriaged labels May 19, 2021
@CEHENKLE
Copy link
Member

@setiah @peternied Thoughts?

@saratvemulapalli
Copy link
Member

@jainankitk taking a stab at this.
Are you looking for client benchmark docs?

Let me start answering this question:

  1. We do have Java docs for benchmarks. See if this helps and let us know if this is what you were looking for. https://opensearch.org/javadocs/1.0.0/OpenSearch/benchmarks/build/docs/javadoc/index.html
  2. Second part of the question, I see you suggest having a pre-check for java docs before the PR is allowed to merge.
    This needs a follow up as I am not sure if precommit already covers it, if not we could add it.

@minalsha minalsha assigned ryanbogan and unassigned ryanbogan Oct 6, 2021
@minalsha
Copy link
Contributor

@jainankitk can you please share your comments on #2 prposed by @saratvemulapalli here: #724 (comment)

@dagneyb
Copy link

dagneyb commented Nov 15, 2022

@jainankitk please let us know if this is resolved with @saratvemulapalli feedback. If we dont hear back by Nov 22nd, we'll consider this resolved. Thanks!

@dagneyb
Copy link

dagneyb commented Nov 22, 2022

Closing as we have not heard back.

@dagneyb dagneyb closed this as completed Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Build Libraries & Interfaces discuss Issues intended to help drive brainstorming and decision making Priority-Low
Projects
None yet
Development

No branches or pull requests

6 participants