Skip to content

docs: Cleanup more literals in API docs#22174

Merged
htuch merged 1 commit intoenvoyproxy:mainfrom
phlax:no-name-config
Jul 15, 2022
Merged

docs: Cleanup more literals in API docs#22174
htuch merged 1 commit intoenvoyproxy:mainfrom
phlax:no-name-config

Conversation

@phlax
Copy link
Copy Markdown
Member

@phlax phlax commented Jul 13, 2022

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

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link
Copy Markdown

Docs for this Pull Request will be rendered here:

https://storage.googleapis.com/envoy-pr/22174/docs/index.html

The docs are (re-)rendered each time the CI envoy-presubmit (precheck docs) job completes.

🐱

Caused by: #22174 was opened by phlax.

see: more, trace.

@phlax phlax changed the title docs: General cleanups [WIP] docs: General cleanups Jul 13, 2022
@phlax phlax marked this pull request as draft July 13, 2022 14:15
Comment thread examples/cache/front-envoy.yaml Outdated
@phlax phlax force-pushed the no-name-config branch 2 times, most recently from 578419e to 3e5f92b Compare July 13, 2022 15:36
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #22174 was synchronize by phlax.

see: more, trace.

Comment thread docs/root/configuration/http/http_filters/aws_lambda_filter.rst Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this covered by the RST warnings? Is there a style guide entry here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

how could it - this is me editing prose to make words that should be literal, so - until we get AI in the pipeline this is not something we can test for

more generally - the use of * for literals relates to historic advice which i changed long ago

we could perhaps check for *word* and spellcheck the word as literals should not be marked up this way - it would catch some - ill look at it when i get to the spellcheck stuff

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess an alternative would be to clarify it in https://github.com/envoyproxy/envoy/blob/main/api/STYLE.md, but as long as we don't detect this in tooling, it will be hard to catch.
Can we hook a validation to the comments parser somehow?

Copy link
Copy Markdown
Member Author

@phlax phlax Jul 14, 2022

Choose a reason for hiding this comment

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

Can we hook a validation to the comments parser somehow?

this is what we do now for single backticks

for * or bare literals its much harder to detect - there are genuine uses of * - ie for italics/emphasis - and there is nothing to grep for with bare literals

spellchecking could work and catch most issues but not all and with some false +ves etc

this is something i intend to work, but not imminently - some preparatory work has been done over in pytooling tho

wrt to the STYLE guide i can update/review if you think it needs it but would suggest we follow up with that

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

also - mho is that the best style guide is what is there - the more we clean stuff up the more people are working from examples that exemplify what we want

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also - mho is that the best style guide is what is there - the more we clean stuff up the more people are working from examples that exemplify what we want

I also tend to follow other examples.

Comment thread docs/root/configuration/http/http_filters/aws_lambda_filter.rst Outdated
@phlax phlax changed the title [WIP] docs: General cleanups docs: Cleanup more literals in API docs Jul 14, 2022
@phlax phlax added this to the 1.23.0 milestone Jul 14, 2022
@phlax phlax marked this pull request as ready for review July 14, 2022 09:28
Signed-off-by: Ryan Northey <ryan@synca.io>
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa 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!
@htuch for a final say.

@phlax
Copy link
Copy Markdown
Member Author

phlax commented Jul 14, 2022

one thing im thinking - we could do with a "literals" policy (maybe that should be literals)

my rule of thumb is "would this be localized" - but that doesnt always help

there are some grey areas, like HTTP or HTTP - which on the basis of it being more acronym than literal it probably should not be a literal - but im not sure if that is ever localized any other way (checking eg https://pontoon.mozilla.org/machinery/)

tldr - i think we just need to agree some rules/priorites regarding literals and code blocks and put those in the style guide - but lets follow up with that

Copy link
Copy Markdown
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.

Thanks for the cleanup. I do think it would be ideal to add at least a style guide entry in a followup, since this is the kind of thing that will regress. Examples are powerful, but some guidance is even better :P

@htuch htuch merged commit 2222039 into envoyproxy:main Jul 15, 2022
silverstar194 added a commit to silverstar194/envoy that referenced this pull request Jul 20, 2022
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: silverstar195 <seanmaloney@google.com>

# Conflicts:
#	api/envoy/config/route/v3/route_components.proto
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.

3 participants