Reworked Go quick start #183
Conversation
|
@dfawley, reassign as appropriate. |
02e83f5
to
fe595a4
Compare
content/docs/quickstart/go.md
Outdated
| 5. Setup the example as a module: | ||
|
|
||
| If things go smoothly, you will see the `Greeting: Hello world` in the client side output. | ||
| ```sh | ||
| $ go mod init examples/helloworld | ||
| ``` | ||
|
|
||
| 6. Adjust import paths to reference local packages (rather than those from the | ||
| original `google.golang.org/grpc` module): | ||
|
|
||
| ```sh | ||
| $ perl -pi -e 's|google.golang.org/grpc/||g' greeter_{client,server}/main.go | ||
| ``` |
There was a problem hiding this comment.
@dfawley - it would be great if the original example sources were already setup in this way, so that we can avoid these last two steps (steps 5 and 6). Might that be possible?
There was a problem hiding this comment.
I missed this comment somehow. So make the import paths relative? I think that would be OK if it's not considered an anti-pattern (we always use absolute paths).
There was a problem hiding this comment.
My concern is that having the reader follow 6 steps just to copy an example's source seems like a lot of work -- especially for a quick start :). Is there any way that we can avoid some of these 6 steps?
My original suggestion what that the helloworld example be defined as a separate module already (named, say, examples/helloworld) in the grpc-go repo, but I don't know if it makes sense to do so (that is, to define a module inside of a module).
There was a problem hiding this comment.
To be fair, one whole step is changing directories.
Is there a curl command we can use to fetch the examples from github? That would avoid the need to chmod them.
The "Download" link on any directory in github links you to something like this: https://download-directory.github.io/?url=https://github.com/grpc/grpc-go/tree/master/examples/helloworld
But then it asks you for a token. 😒
My original suggestion what that the helloworld example be defined as a separate module already (named, say, examples/helloworld) in the grpc-go repo, but I don't know if it makes sense to do so (that is, to define a module inside of a module).
Yes, it's totally OK to have modules inside other modules. Such a change is a breaking change, though. But this is just an example. And there's some amount of shenanigans you may need to do to avoid the conflict of older versions of grpc-go having the same packages. I think you do this by setting the sub-module to require a version of grpc-go after (or at the same commit where) the sub-module was forked. But I'm not able to find the right documentation right now (I'll keep searching).
There was a problem hiding this comment.
Of course, after 5 minutes of failed searches....then my first query after sending this turned up what I was looking for:
There was a problem hiding this comment.
Yes since you're altering the path to drop the "/grpc/" part. Without step 6 the code would refer to the .pb.go files from the official grpc module.
There was a problem hiding this comment.
"Yes" step 6 would be unnecessary? Or "Yes" it would still be necessary?
If the former, we can just make that change and not do anything in the grpc-go repo (I think?).
There was a problem hiding this comment.
Oops, apologies for being unclear in my answer. Step 6 would still be necessary because go mod init google.golang.org/examples/helloworld essentially changes the path to the example. The quick start also (now) changes the path to the example.
Btw, did you mean to write: go mod init google.golang.org/grpc/examples/helloworld -- which would preserve the path to the example, merely making helloworld an independent module? In that case we run into the ambiguous import problem.
Might you want us to discuss this (well, grpc/grpc-go#3540, actually) over a VC some time tomorrow?
There was a problem hiding this comment.
Btw, did you mean to write: go mod init google.golang.org/grpc/examples/helloworld -- which would preserve the path to the example, merely making helloworld an independent module? In that case we run into the ambiguous import problem.
Yes, that's what I meant, sorry. OK, I see we still would also need a go.mod inside the grpc repo so that it removes those paths from its module.
At this point, given all the constraints I can see, and the options you laid out in grpc/grpc-go#3540, what I'd prefer is adding a single go.mod to grpc/examples (3), and having the quick start guide recommend copying all of grpc/examples elsewhere to work on. If that SGTY then I don't think there is any need to meet. If you would prefer another direction or just want to explore all/some of the options further, then I'm happy to meet and am available tomorrow. Feel free to email me (or use hangouts) and we can set up a time.
There was a problem hiding this comment.
I answered in grpc/grpc-go#3540 (comment) -- so that we could move the discussion back over there.
32397d3
to
17b0f54
Compare
|
@dfawley - ETA on a review? |
|
@dfawley - btw, we have a "gRPC Docs Working Group" GVC at 13:30 ET in case you'd like to join. Or, if you prefer to have a focused discussion concerning this PR with me, I'm available for a VC. Otherwise, comments attached to this PR would be fine. Let me know what would be simplest for you. |
17b0f54
to
796a80c
Compare
I had already started to review this, so I finished up & sent my comments. I can join the GVC if you'd like to discuss anything significant, but I didn't have any major comments, so I think the PR should be sufficient. Thanks! |
|
Thanks for the review! I'll work from your comments (probably after the meeting). |
I’d love to join! Is there an invitation? |
|
@zacharysarah - I've asked @jtattermusch to add you to the weekly meeting invite. |
|
@dfawley - done with changes PTAL |
efee165
to
a31667b
Compare
|
@lucperkins - did you want to review this? |
a31667b
to
cf45cd4
Compare
|
Thanks for the approval @dfawley! |
Might help alleviate #177?
@srini100 @jtattermusch @ejona86 - who best should be assigned to review this?
cc @lucperkins @zacharysarah @thisisnotapril