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.

Sign up for GitHub

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

Jump to bottom

docs: Update start/install section #13490

Merged
merged 59 commits into from
Oct 24, 2020
Merged

docs: Update start/install section #13490

merged 59 commits into from
Oct 24, 2020

Conversation

phlax
Copy link
Member

@phlax phlax commented Oct 10, 2020 •
edited
Loading

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: docs: Update start/install section
Additional Description:

This PR will

Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Runtime guard:]
[Optional Fixes #Issue] Fix #13408 Fix #13491 touch #13461 touch #13439
[Optional Deprecated:]

@phlax phlax marked this pull request as draft October 10, 2020 16:09
@phlax phlax force-pushed the docs-update-install branch from c040807 to 3fe85ab Compare October 10, 2020 16:38
@mattklein123 mattklein123 self-assigned this Oct 10, 2020
@phlax
Copy link
Member Author

phlax commented Oct 10, 2020

after doing a bit of initial moving around i think it looks good:

https://389760-65214191-gh.circle-artifacts.com/0/generated/docs/start/start.html

im wondering about the ref_configs page:

https://389760-65214191-gh.circle-artifacts.com/0/generated/docs/start/ref_configs.html

specifically, im wondering whether the sandboxes are kinda ref configs, and we should move the "Configuration generator" into tools

@phlax phlax force-pushed the docs-update-install branch from 72abce9 to 780edde Compare October 11, 2020 11:05
@phlax phlax changed the title [WIP] docs: Update install section [WIP] docs: Update start/install section Oct 11, 2020
@phlax phlax mentioned this pull request Oct 11, 2020
Closed
@phlax phlax force-pushed the docs-update-install branch from cf399d4 to 9f3aad4 Compare October 11, 2020 16:12
@mattklein123
Copy link
Member

specifically, im wondering whether the sandboxes are kinda ref configs, and we should move the "Configuration generator" into tools

Yeah I was going to discuss this with you. IMO we should merge configs/examples in the repo, and probably just move the config generator into examples? I think this would make more sense since what we have in configs at this point is really just another type of example.

Is this ready for review or still WIP?

@phlax
Copy link
Member Author

phlax commented Oct 11, 2020

Is this ready for review or still WIP?

still WIP im afraid - i have almost finished the install and docker pages:

so its worth having a look over them and the overall layout so far, but im still working on the rest

@phlax
Copy link
Member Author

phlax commented Oct 11, 2020

im minded to land the "default configuration for docker" section as is for now

https://390000-65214191-gh.circle-artifacts.com/0/generated/docs/start/docker.html#running-envoy-docker-with-the-default-configuration

and tidy up after (re #13497)

@phlax
Copy link
Member Author

phlax commented Oct 11, 2020

Yeah I was going to discuss this with you. IMO we should merge configs/examples in the repo, and probably just move the config generator into examples? I think this would make more sense since what we have in configs at this point is really just another type of example.

i had thought to move it to tools - let me update and we can figure from there

@phlax
Copy link
Member Author

phlax commented Oct 11, 2020

i see now - you mean move the actual ref configs to sandboxes - yep, i think thats a good idea.

i had only started reading the "config generator" part, which i think can go to tools

@phlax
Copy link
Member Author

phlax commented Oct 11, 2020

hmm, this is what the ref_configs point to https://390000-65214191-gh.circle-artifacts.com/0/generated/docs/intro/deployment_types/deployment_types.html

i think we might want to look at the intro section after, but for now i have just remove the page from the start section and redirected here (i can do something different)

@phlax phlax force-pushed the docs-update-install branch 2 times, most recently from 205aca6 to 02af108 Compare October 12, 2020 14:58
@phlax
Copy link
Member Author

phlax commented Oct 12, 2020 •
edited
Loading

@mattklein123 this is the last build on circleci https://390172-65214191-gh.circle-artifacts.com/0/generated/docs/start/start.html

the install and docker sections are almost complete, and im half way through the quick-start (text will be readded)

probs worth looking over them as there are a lot of changes - trying to figure how i can add doc artifacts asap... (re #13506 )

mattklein123
mattklein123 requested changes Oct 12, 2020
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Overall this is amazing! I love it. Left a few small comments. LMK when it's ready for full review.

/wait

docs/root/intro/deployment_types/double_proxy.rst Outdated Show resolved Hide resolved
docs/root/start/start.rst Outdated Show resolved Hide resolved
docs/root/start/tools/config_generator.rst Outdated Show resolved Hide resolved
@phlax phlax mentioned this pull request Oct 13, 2020
Closed
@phlax
Copy link
Member Author

phlax commented Oct 14, 2020

@mattklein123 ive created a PR to deal with the Lyft references separately #13559

@phlax phlax force-pushed the docs-update-install branch from 57bbd95 to 3e22cca Compare October 14, 2020 09:15
@phlax phlax mentioned this pull request Oct 14, 2020
Merged
@phlax phlax force-pushed the docs-update-install branch from 3e22cca to 7fd9a7a Compare October 14, 2020 19:18
phlax added 2 commits October 23, 2020 17:03
@phlax
finishing touches to install page
7e7d2a6 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
docs/
d5da910 
Signed-off-by: Ryan Northey <ryan@synca.io>
@mattklein123
Copy link
Member

Awesome, suggest we ship and iterate from here. Can you merge main?

/wait

@phlax phlax force-pushed the docs-update-install branch from 2d7dcfa to d5da910 Compare October 23, 2020 16:04
@phlax
Copy link
Member Author

phlax commented Oct 23, 2020

cool, yep, just did, and fixing redirects

@phlax
Copy link
Member Author

phlax commented Oct 23, 2020

@mattklein123 i think we need to wait for tests

@mattklein123
Copy link
Member

@mattklein123 i think we need to wait for tests

SG. Also, friendly request to please not force push. It makes reviews a lot more difficult.

mattklein123
mattklein123 previously approved these changes Oct 23, 2020
View reviewed changes
@phlax
Copy link
Member Author

phlax commented Oct 23, 2020

SG. Also, friendly request to please not force push. It makes reviews a lot more difficult.

apologies, im more used to doing that to resolve confllicts. ill try to rem not to

on a related not see #13735

@phlax
Copy link
Member Author

phlax commented Oct 23, 2020

aaggh - #13735 is causing master fail

@phlax
docs/
80554ab 
Signed-off-by: Ryan Northey <ryan@synca.io>
phlax added 8 commits October 23, 2020 18:12
@phlax
docs/
f6a0f97 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
docs/
fa617cb 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
docs/
b57691c 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
docs/
f2ffbd3 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
docs/
a883a30 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
docs/
d23fa9f 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
docs/
658b154 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
docs/
25764fc 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Oct 23, 2020

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines.

🐱

Caused by: a #13490 (comment) was created by @phlax.

see: more, trace.

@mattklein123 mattklein123 merged commit 51dc07d into envoyproxy:master Oct 24, 2020
@phlax phlax mentioned this pull request Oct 26, 2020
Open
This was referenced Oct 31, 2020
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

Assignees

@mattklein123 mattklein123

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

brew install envoy Improve documentation around running Envoy in Docker container
2 participants
@phlax @mattklein123