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

protodoc: Add redirects for v2 protos to current version and links to old #16303

Merged
merged 19 commits into from
May 7, 2021

Conversation

phlax
Copy link
Member

@phlax phlax commented May 4, 2021

Commit Message: protodoc: Add redirects for v2 protos to current version and links to old
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue] Fix #16297
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #16303 was opened by phlax.

see: more, trace.

@phlax
Copy link
Member Author

phlax commented May 4, 2021

@phlax phlax force-pushed the protodoc-v2-redirects branch from 8eb7427 to 28fd4dd Compare May 4, 2021 12:04
.azure-pipelines/pipelines.yml Outdated Show resolved Hide resolved
docs/build.sh Outdated Show resolved Hide resolved
docs/build.sh Outdated Show resolved Hide resolved
tools/protodoc/protodoc.py Outdated Show resolved Hide resolved
@phlax phlax force-pushed the protodoc-v2-redirects branch 2 times, most recently from 3165ba0 to fb545f8 Compare May 4, 2021 19:24
tools/protodoc/protodoc.py Outdated Show resolved Hide resolved
@phlax phlax force-pushed the protodoc-v2-redirects branch from fabed64 to c4b3a1e Compare May 4, 2021 20:00
docs/build.sh Show resolved Hide resolved
@phlax phlax force-pushed the protodoc-v2-redirects branch 5 times, most recently from f063b4a to 230e00d Compare May 5, 2021 10:03
@phlax phlax changed the title [WIP] protodoc: Add redirects for v2 protos to current version protodoc: Add redirects for v2 protos to current version May 5, 2021
@phlax phlax changed the title protodoc: Add redirects for v2 protos to current version protodoc: Add redirects for v2 protos to current version and links to old May 5, 2021
@phlax phlax marked this pull request as ready for review May 5, 2021 11:33
@phlax phlax requested a review from htuch May 5, 2021 11:33
tools/protodoc/protodoc.py Outdated Show resolved Hide resolved
tools/protodoc/protodoc.py Outdated Show resolved Hide resolved
docs/build.sh Outdated Show resolved Hide resolved
docs/build.sh Show resolved Hide resolved
phlax added 3 commits May 6, 2021 09:12
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
@phlax
Copy link
Member Author

phlax commented May 6, 2021

this PR now requires jq which i think is a good thing to add in general

I have hacked it in here for now - but ive also raised a PR in envoy-build-tools to add envoyproxy/envoy-build-tools#132

there is alternately a bazel recipe i found here https://github.com/kythe/kythe/blob/master/third_party/jq.BUILD

im happy to resolve in any of the above ways - as long as we can add it

docs/build.sh Outdated Show resolved Hide resolved
phlax added 2 commits May 6, 2021 09:43
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
ci/run_envoy_docker.sh Outdated Show resolved Hide resolved
phlax added 4 commits May 6, 2021 10:33
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
@htuch
Copy link
Member

htuch commented May 6, 2021

Code looks good. Looking at the docs, the box on the RHS is a bit of an eye sore. I know you are thinking of tabs here, is there any way to make the docs less cluttered before landing them?

@phlax
Copy link
Member Author

phlax commented May 6, 2021

Code looks good. Looking at the docs, the box on the RHS is a bit of an eye sore. I know you are thinking of tabs here, is there any way to make the docs less cluttered before landing them?

this is the best i could come up with before shifting things to tabs - it needs to work in a few different situtations

this is far from ideal - but i would like to get this landed and then work on the tabs - which will take some time in terms of figuring out how best to render - im working on it already tho...

@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #16303 was synchronize by phlax.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label May 6, 2021
Signed-off-by: Ryan Northey <[email protected]>
@phlax
Copy link
Member Author

phlax commented May 7, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

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

see: more, trace.

phlax added 4 commits May 7, 2021 14:46
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I think the current UX is fine for interim, looking forward to a tabs focused replacement.

@htuch
Copy link
Member

htuch commented May 7, 2021

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label May 7, 2021
@htuch htuch merged commit 85a8570 into envoyproxy:main May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Envoy documentation SEO links partially broken
3 participants