examples: Added README files for all missing Examples#11676
examples: Added README files for all missing Examples#11676shivaspeaks merged 23 commits intogrpc:masterfrom
Conversation
| @@ -135,6 +136,24 @@ before trying out the examples. | |||
|
|
|||
| - [Keep Alive](src/main/java/io/grpc/examples/keepalive) | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I can see there are 21 examples, working on this comment and its in progress
There was a problem hiding this comment.
@kannanjgithub @shivaspeaks Added the Readme file with details for the all examples and its ready for Review. please have look once.
There was a problem hiding this comment.
@kannanjgithub @shivaspeaks Added the Readme file with details for the all examples and its ready for Review. please have look once.
@kannanjgithub @shivaspeaks This PR is pending for review from long time, Request you have a look once.
|
I see this says |
@ejona86 Thanks for your reply, I was refering to one of the change as the "directory structure" for example-alts as it is not consistant with all the other examples projects directory so made this change/move as below in this PR apart from this no other changes related to directory structure. "GRPC Example documentation" was refering here only for adding new README files in all examples with explanations nothing else. |
|
Sigh... my comment #11676 (comment) still applies. This doesn't fix #11676 and the directory structure it was talking about it not related to this change. |
|
Oh, but I thought the need for those changes was striked out by your comment on #5467 (comment)
If not, perhaps we can reopen #5467 and edit the issue description with what's left. |
|
@shivaspeaks, I see that you improved the PR description, but that didn't get copied into the commit itself. That information should generally be in the commit; the only information I tend to omit in the commit message is messages to the reviewer (e.g., "This is the first part; I will send out the second part next week.") Managing that is much easier for your own PRs as you can write a good commit message and have github auto-copy it to the PR description. But if the review changes the code substantially or when you merge someone else's change, it is good to double-check the commit message. I do think the example structure could be improved, but #5467 didn't really say what was wrong or what would be better and it is low-quality, mixing of multiple things, and confusing enough that a replacement issue would maybe be better. FWIW, that needed discussion/design for what would be better. The issue was "this needs improving" and I agree with that, but that doesn't mean the solution is fully-formed. (There are some examples, like the json one, that should be moved to its own directory because they have extra dependencies. But what changes to do where really requires an audit to look through it and see what could be improved.) |

Added new README files for all the examples with details and changed the incorrectness/ typo in path of
examples/example-alts/example-alts/README.md→examples/example-alts/README.mdFixes #5467