Skip to content

tools: format checks for backticks#17566

Merged
alyssawilk merged 3 commits intoenvoyproxy:mainfrom
alyssawilk:ticks
Aug 3, 2021
Merged

tools: format checks for backticks#17566
alyssawilk merged 3 commits intoenvoyproxy:mainfrom
alyssawilk:ticks

Conversation

@alyssawilk
Copy link
Contributor

Risk Level: n/a (tooling only)
Testing: against current, 1.19.0
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
# Make sure backticks come in pairs.
# Exceptions: reflinks (ref:`` where the backtick won't be preceded by a space
# links `title <link>`_ where the _ is checked for in the regex.
BAD_TICKS_REGEX = re.compile(r".* `[^`].*`[^_]")
Copy link
Member

Choose a reason for hiding this comment

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

im wondering will this work across lines ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should - AFIK the rst script collects a line's worth of data in line and only then does checks like "does this release note have a terminal period"

It will miss things like (header name) so it's still best effort but IMO better than nothing.

Copy link
Member

Choose a reason for hiding this comment

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

yep, for sure

the reason i hadnt added this previously is that a line regex wont catch situations like

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas nec porttitor tellus, commodo tincidunt elit. ``Nullam
auctor neque`` pulvinar ipsum congue, a lacinia nulla sagittis.

which i think is valid rst etc

my vague intention on this was to add a sphinx plugin that hooked in to the "default role" (i think that is what its called in rstspeak) and errored

but, yep, this should catch most i think

Copy link
Member

@phlax phlax 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 @alyssawilk

i was gonna ask why it wasnt erroring with so many of these in the code base - and then i remmed that this is only checking this one file atm

i can follow up to look at enabling this for more/the rest of the rst code

@alyssawilk alyssawilk merged commit a8a2eb9 into envoyproxy:main Aug 3, 2021
baojr added a commit to baojr/envoy that referenced this pull request Aug 4, 2021
…bridge-stream

* upstream/main: (32 commits)
  tls: move ssl connection info into SocketAddressProvider (envoyproxy#17334)
  conn pool: default enable runtime feature `conn_pool_delete_when_idle` (envoyproxy#17577)
  api: LEDS api introduction (envoyproxy#17419)
  kafka: add support for api versions request in mesh-filter (envoyproxy#17475)
  ext_proc: Implement BUFFERED_PARTIAL processing mode (envoyproxy#17531)
  tooling: Async/pathlib/mypy cleanups and utils (envoyproxy#17505)
  xds: restructure CertificateProvider fields (envoyproxy#17201)
  Refactor OverloadIntegrationTest breaking out a test base, and the fake resource monitors. (envoyproxy#17530)
  listener: move active connection collection out of active tcp listener (envoyproxy#16947)
  tools: format checks for backticks (envoyproxy#17566)
  coverage: set lower limit for common/quic and common (envoyproxy#17573)
  v2: final source removal (envoyproxy#17565)
  test: bumping coverage (envoyproxy#17564)
  quic: enforcing header size and contents (envoyproxy#17520)
  Support for canonicalizing URI properly for AWS SigV4 signer (envoyproxy#17137)
  listener: add a stat for transport socket connect timeout (envoyproxy#17458)
  listener: add listen() error handling (envoyproxy#17427)
  http: return per route config when direct response is set (envoyproxy#17449)
  removing most v2 references from source/ (envoyproxy#17415)
  bug fix: return bootstrap when validating config (envoyproxy#17499)
  ...

Signed-off-by: Garrett Bourg <bourg@squareup.com>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Risk Level: n/a (tooling only)
Testing: against current, 1.19.0
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk deleted the ticks branch August 4, 2022 00:57
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