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.

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

docs: Move envoy config snippets to include files #13231

Merged

Conversation

phlax
Copy link
Member

@phlax phlax commented Sep 23, 2020

Signed-off-by: Ryan Northey [email protected]

Commit Message: docs: Move snippets to include files
Additional Description:

this moves any (lintable) snippets currently embedded in docs rst files into include files.

this will:

  • make linting config and/or code snippets much easier in CI/locally
  • make it easier to use code editors etc when editing the snippets
  • make it easier to edit the copy and structure of rst files

Risk Level: low
Testing: potentially
Docs Changes: yep
Release Notes:
[Optional Runtime guard:]
[Optional Fixes #Issue] touch #8837
[Optional Deprecated:]


update to scope

after looking through the docs - i realised that there are quite a lot of partial snippets. I have avoided these and just moved out the (full) envoy yaml snippets.

i think we might want to move the partial snippets after (and perhaps some non-yaml ones)

there are some funky things we can do with partial snippets - like add context and highlight certain lines, or sequentially show parts of an included full config, but all of these things are a bit more complex - so getting lintable/testable envoy configs out seems a greater priority.

also, im seeing that there is a validated-code-block extension - so i need to look at that more.

@phlax phlax marked this pull request as draft September 23, 2020 14:53
@phlax phlax changed the title [WIP] docs: Move snippets to include files [RFC/WIP] docs: Move snippets to include files Sep 23, 2020
@phlax
Copy link
Member Author

phlax commented Sep 23, 2020

@mattklein123 as this means moving quite a bit of config - i just did the first one i found for testing - if you are happy with this general direction - ill move the rest/more

@mattklein123 mattklein123 self-assigned this Sep 23, 2020
@mattklein123
Copy link
Member

Yeah this is totally fine with me. Whatever you think is best to getting to a path where everything is validated and won't bit rot. (See also the inline validation code that already exists that I mentioned elsewhere.)

Let me know when this is ready to review.

/wait

@phlax phlax changed the title [RFC/WIP] docs: Move snippets to include files [WIP] docs: Move snippets to include files Sep 23, 2020
@phlax phlax force-pushed the docs-move-lintable-snippets-to-includes branch from 4e1e80c to a9a5d37 Compare September 25, 2020 07:48
@phlax phlax changed the title [WIP] docs: Move snippets to include files [WIP] docs: Move envoy config snippets to include files Sep 25, 2020
@phlax phlax changed the title [WIP] docs: Move envoy config snippets to include files docs: Move envoy config snippets to include files Sep 25, 2020
@phlax phlax marked this pull request as ready for review September 25, 2020 10:42
@phlax
Copy link
Member Author

phlax commented Sep 25, 2020

@mattklein123 i have updated the scope (see description) and deWIPped this

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.

Nice, thanks.

@mattklein123 mattklein123 merged commit 1c27396 into envoyproxy:master Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants