Improve scripts/pr_file_map.py observability and output metadata - #15317
Conversation
Co-authored-by: cclauss <3709715+cclauss@users.noreply.github.com>
Co-authored-by: cclauss <3709715+cclauss@users.noreply.github.com>
Co-authored-by: cclauss <3709715+cclauss@users.noreply.github.com>
Co-authored-by: cclauss <3709715+cclauss@users.noreply.github.com>
scripts/pr_file_map.py observability and output metadata
Co-authored-by: Christian Clauss <cclauss@me.com>
|
@priya-sundaram-dev, please review. |
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Reviewed — the observability additions are a nice improvement, and CI is green (build/ruff/ty/pre-commit all pass). One real bug though:
In the "missing files" header, the f prefix got dropped:
print("\n## `{missing_count}` files not present in the working directory\n")Because it's a plain string (not an f-string), this renders the literal text `{missing_count}` instead of the count — unlike the "existing files" header just above it, which is correctly an f-string. Fix:
print(f"\n## `{missing_count}` files not present in the working directory\n")
Two small optional notes:
missing_countis computed before theif pr_count == 0: returnguard, so it's always defined by the time it's used here — good.- Consider mentioning the new stderr status lines in the
Usage:block of the docstring so users know to expect them on stderr (they won't pollute the redirected markdown, which is the right call).
With the f-string fix this LGTM.
|
Reviewed post-merge — the changes read well and are a nice observability upgrade. The One small follow-up (non-blocking, since it's already in):
|
|
@priya-sundaram-dev Nice call. Please create a pull request to improve the script in ways that improve our collaboration on landing pull requests. |
|
Done — opened #15321. It adds a files-touched-by-more-than-one-open-PR section up top (most-contested files first) so overlapping PRs are obvious when deciding what to land, and splits the ambiguous file count into "file touches" vs "distinct files touched" (the latter now matches existing + missing). Only |
Describe your change
pr_file_map.pynow emits structured run-status counts to stderr and includes generation metadata plus summary counts in markdown output. It also migrates path existence checks fromos.pathtopathlib.Pathand documents the updated behavior in the module docstring.Path handling
os.path.exists(path)withPath(path).exists().Runtime status (stderr)
get_open_prs()get_pr_files()Report output (stdout markdown)
Path(__file__).resolve())Checklist