-
Notifications
You must be signed in to change notification settings - Fork 64
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
Couple doc updates #101
Couple doc updates #101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@yaron2 ready for your approval |
@calebcartwright could you please resolve the conflict |
Will close #78 |
Will do so later today, apologies for the delay |
Signed-off-by: Caleb Cartwright <[email protected]>
Signed-off-by: Caleb Cartwright <[email protected]>
6231e56
to
1fcb3e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
I almost forgot to say thank you to you @calebcartwright for raising the PR and bringing clarity to the docs. Much appreciated! 🙌 |
Two minor suggested updates to the docs:
prost
no longer bundlesprotoc
and will no longer attempt to download the compiler if one doesn't already exist. Users will need protoc installed to work with the SDK so worth noting that as a pre-req IMOvendor
directory for 3rd party Go modules which seemed out of place to me and a potential copy/paste leftover from another repoNote that I opted to make these separate commits particularly in case I'm missing some context around that
vendor
directory and 6231e56 is not desired, but I can certainly squash the changes together if you'd prefer both in a single commit