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

examples: GRPC Examples documentation and Consistency on directory structure changes #11676

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

vinodhabib
Copy link
Contributor

@vinodhabib vinodhabib commented Nov 8, 2024

Changes for GRPC Examples documentation and Consistency on Directory Structure

Fixes #5467

@vinodhabib vinodhabib marked this pull request as draft November 8, 2024 06:43
@vinodhabib vinodhabib marked this pull request as ready for review November 8, 2024 12:57
@vinodhabib vinodhabib marked this pull request as draft November 8, 2024 12:57
@vinodhabib vinodhabib changed the title examples: following the consistency for example-alts directory structure examples: GRPC Examples documentation and Consistency on directory structure changes Nov 11, 2024
@vinodhabib vinodhabib marked this pull request as ready for review November 11, 2024 12:36
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved

- [Load Balance](src/main/java/io/grpc/examples/loadbalance)

- [Multiplex](src/main/java/io/grpc/examples/multiplex)
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a paragraph of explanation for non-trivial examples? As done for Retrying, Health Service, etc.

Copy link
Contributor Author

@vinodhabib vinodhabib Nov 12, 2024

Choose a reason for hiding this comment

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

not sure on this part as this is an existing one and can see the paragraph of explanation for only 3 examples not for all which are existing previously (except the newly added changes).
I can also see some doc comments in the implementation classes are giving some of the details.

@@ -135,6 +136,24 @@ before trying out the examples.

- [Keep Alive](src/main/java/io/grpc/examples/keepalive)
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not clear to me which part of Sanjay Pujare's concerns this PR addresses.
"each example should have a README file which a user can read to understand the example." I don't find this done, for example there is no README for the subdirectories in https://github.com/grpc/grpc-java/tree/master/examples/src/main/java/io/grpc/examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kannanjgithub @shivaspeaks There is a common README file for all the examples (which are inside the path https://github.com/grpc/grpc-java/tree/master/examples/src/main/java/io/grpc/examples) as mentioned in the below link and updated some of the missing ones with recent changes as part of this PR.

Common README link in grpc-java - https://github.com/grpc/grpc-java/blob/master/examples/README.md

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue mentions "each example should have a README file which a user can read to understand the example." Merely enumerating the examples in the sub-directories to the parent README does not do that.

Copy link
Contributor Author

@vinodhabib vinodhabib Nov 19, 2024

Choose a reason for hiding this comment

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

I can see there are 21 examples, working on this comment and its in progress

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.

Enhancements to examples
3 participants