Skip to content

ci: validate internal doc comments match markdown documentation #32074

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

petrosagg
Copy link
Contributor

Motivation

This is a follow up PR from my previous work that will ensure the field documentation we publish in our docs is also reflected as comments in the product. Having it as a CI step will ensure the two will stay in sync.

Tips for reviewer

The last commit is pure code gen and is marked as such

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@petrosagg petrosagg requested review from a team as code owners April 2, 2025 13:33
@petrosagg petrosagg requested a review from ParkMyCar April 2, 2025 13:33
@petrosagg petrosagg force-pushed the ci-validate-doc-comments branch from 6bce7f8 to 683da24 Compare April 2, 2025 14:02
Copy link
Member

@antiguru antiguru left a comment

Choose a reason for hiding this comment

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

Looks good, thank you! Left a comment inline.

@@ -106,7 +124,8 @@ def main() -> None:
elif "array" in type_name:
type_name = "array"
type_name = type_name.replace(" ", "␠")
print(" ".join([str(position), field, type_name]))
documentation = documentation.replace(" ", "␠")
print(" ".join([field, type_name, documentation]))
position += 1
Copy link
Member

Choose a reason for hiding this comment

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

Probably don't need position anymore?

Copy link
Contributor

@kay-kim kay-kim left a comment

Choose a reason for hiding this comment

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

😍

field = FIELD_NAME_RE.search(fields[0]).group(1)
type_name = FIELD_TYPE_RE.search(fields[1]).group(1)
documentation = DOC_LINK_TYPE1_RE.sub(r"\1", fields[2])
documentation = DOC_LINK_TYPE2_RE.sub(r"\1", documentation)
Copy link
Contributor

@def- def- Apr 2, 2025

Choose a reason for hiding this comment

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

Why is documentation being assigned twice? Edit: I guess it makes sense, just doing two substitutions. I would have just nested them.

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.

5 participants