Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ rsync -av \
"${SCRIPT_DIR}"/_ext \
"${GENERATED_RST_DIR}"

bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/code_format:rst_check "${GENERATED_RST_DIR}"

# To speed up validate_fragment invocations in validating_code_block
bazel build "${BAZEL_BUILD_OPTIONS[@]}" //tools/config_validation:validate_fragment

Expand Down
6 changes: 6 additions & 0 deletions tools/code_format/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,9 @@ envoy_py_binary(
requirement("yapf"),
],
)

py_binary(
name = "rst_check",
srcs = ["rst_check.py"],
visibility = ["//visibility:public"],
)
36 changes: 36 additions & 0 deletions tools/code_format/rst_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/python3

import subprocess
import sys

# TODO(phlax): add rstcheck also

# things we dont want to see in generated docs
# TODO(phlax): move to .rstcheck.cfg when available
Copy link
Member

Choose a reason for hiding this comment

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

You have another WiP PR here; a bit confused..

Copy link
Member Author

Choose a reason for hiding this comment

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

so the point was to move it there once that PR landed

i wanted to land this because it fixes actual problems, and then add a better solution in the follow up #15786

Copy link
Member

Choose a reason for hiding this comment

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

Can you let me know when the Python is ready for final review?

Copy link
Member

Choose a reason for hiding this comment

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

/wait

RSTCHECK_GREP_FAIL = (" ref:", "\\[\\#")


def run_grep_check(check):
Copy link
Member Author

Choose a reason for hiding this comment

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

add function signature

command = ["grep", "-nr", "--include", "\\*.rst"] + [check]
Copy link
Member

Choose a reason for hiding this comment

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

Is the plan to expand this beyond just grep checking? If it's a grep wrapper, then a shell script is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

my plan is to rewrite it with the proper rstcheck (ie #15786 ) and using class like in #15802 so its kinda temporary to do it this way - but it fixes the immediate issue and introduces the generated rst checking

resp = subprocess.run(command, capture_output=True, cwd=sys.argv[1])

# this fails if returncode is 0 - ie it should not have any matches
if not resp.returncode:
# stdout and stderr are dumped to ensure we capture all errors
sys.stderr.write(
f"ERROR: rstcheck linting failed, found unwanted: '{check}'\n"
f"{resp.stdout.decode('utf-8')}\n"
Copy link
Member

Choose a reason for hiding this comment

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

Probably best to factor out the decode stuff for clarity.

f"{resp.stderr.decode('utf-8')}")
return len(resp.stdout.decode("utf-8").split("\n")) - 1


def main():
errors = 0
for check in RSTCHECK_GREP_FAIL:
errors += run_grep_check(check)
if errors:
raise SystemExit(f"RST check failed: {errors} errors")


if __name__ == "__main__":
main()