-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix(docs): Clarify quick start installation. Fixes #13032 #13047
Conversation
59f99c5
to
94f1316
Compare
Apologies, knew I should have opened as a draft but clicked too hastily and missed the little arrow on the button. |
It's not necessary for most of the docs, only the codegen'd ones. CI passes so you're good. |
Great, thanks for confirming! Happy to make any changes or rebase or whatever :) |
Yea I was gonna request some small changes when I was at my computer (previous comments were from my phone). Should be done later today |
Thanks for your time, Anton ❤️ Just want to say I'm really impressed with the community y'all have built here 🤩 |
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.
A few small changes below
I got hung up on these because I copied the commands from the releases page instead of from the quick start page, so I accidentally applied `install.yaml` instead of `quick-start-minimal.yaml`. I hope this change will help others not get confused like me :) Signed-off-by: Matt Fisher <mfisher87@gmail.com>
Signed-off-by: Matt Fisher <mfisher87@gmail.com>
Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com> Signed-off-by: Matt Fisher <mfisher87@gmail.com>
Signed-off-by: Matt Fisher <mfisher87@gmail.com>
Signed-off-by: Matt Fisher <mfisher87@gmail.com>
b5abdac
to
62fd51e
Compare
Signed-off-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
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. Thanks for pointing this out and fixing it!
I got hung up on these instructions because I copied the commands from the releases page instead of from the quick start page, so I applied
install.yaml
instead ofquick-start-minimal.yaml
. I hope this change will help others not get confused like me :)I tried
make pre-commit -B
(had to installgo
first) and it failed withprotoc
not found. Do I need to keep working my way through these, or is it OK to ignore for a docs change like this?Fixes #13032
Motivation
I followed the quick start instructions explicitly, but ended up with an install that couldn't execute Hello World step.
Modifications
Alter docs to explicitly instruct the user to install the quick start manifest.
I also moved a warning admonition because I felt it belonged after prose I added in this PR.
Verification
Rendered the docs, looked good