Skip to content

Commit b900c7f

Browse files
feat: Refactor comment filtering with new status terms and flags
This commit introduces a more granular system for classifying and filtering pull request review comments in the `scripts/gha/get_pr_review_comments.py` script. New Comment Statuses: - `[IRRELEVANT]`: Comment's original diff position is lost (`position` is null). Displays `original_line`. - `[OLD]`: Comment is anchored to the diff, but its line number has changed (`line` != `original_line`). Displays current `line`. - `[CURRENT]`: Comment is anchored and its line number is unchanged. Displays current `line`. New Command-Line Flags: - `--exclude-old` (default False): If set, hides `[OLD]` comments. - `--include-irrelevant` (default False): If set, shows `[IRRELEVANT]` comments (which are hidden by default). - The old `--skip-outdated` flag has been removed. Default Behavior: - Shows `[CURRENT]` and `[OLD]` comments. - Hides `[IRRELEVANT]` comments. This provides you with more precise control over which comments are displayed, improving the script's utility for various review workflows. The "suggest next command" feature correctly interacts with these new filters, only considering non-skipped comments for its timestamp calculation.
1 parent 203e88f commit b900c7f

File tree

1 file changed

+26
-18
lines changed

1 file changed

+26
-18
lines changed

scripts/gha/get_pr_review_comments.py

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,16 @@ def main():
7777
help="Only show comments created at or after this ISO 8601 timestamp (e.g., YYYY-MM-DDTHH:MM:SSZ)."
7878
)
7979
parser.add_argument(
80-
"--skip-outdated",
80+
"--exclude-old",
8181
action="store_true",
82-
help="If set, comments marked [OUTDATED] or [FULLY_OUTDATED] will not be printed."
82+
default=False,
83+
help="Exclude comments marked [OLD] (where line number has changed due to code updates but position is still valid)."
84+
)
85+
parser.add_argument(
86+
"--include-irrelevant",
87+
action="store_true",
88+
default=False,
89+
help="Include comments marked [IRRELEVANT] (where GitHub can no longer anchor the comment to the diff, i.e., position is null)."
8390
)
8491

8592
args = parser.parse_args()
@@ -98,8 +105,7 @@ def main():
98105
sys.stderr.write(f"Fetching comments for PR #{args.pull_number} from {firebase_github.OWNER}/{firebase_github.REPO}...\n")
99106
if args.since:
100107
sys.stderr.write(f"Filtering comments created since: {args.since}\n")
101-
if args.skip_outdated:
102-
sys.stderr.write("Skipping outdated comments based on status.\n")
108+
# Removed skip_outdated message block
103109

104110

105111
comments = firebase_github.get_pull_request_review_comments(
@@ -115,6 +121,7 @@ def main():
115121
latest_created_at_obj = None
116122
print("# Review Comments\n\n")
117123
for comment in comments:
124+
# This replaces the previous status/skip logic for each comment
118125
created_at_str = comment.get("created_at")
119126

120127
current_pos = comment.get("position")
@@ -123,25 +130,25 @@ def main():
123130

124131
status_text = ""
125132
line_to_display = None
126-
is_effectively_outdated = False
127-
128-
if current_pos is None: # Comment's specific diff context is gone
129-
status_text = "[FULLY_OUTDATED]"
130-
line_to_display = original_line # Show original line if available
131-
is_effectively_outdated = True
132-
elif original_line is not None and current_line != original_line: # Comment on a line that changed
133-
status_text = "[OUTDATED]"
134-
line_to_display = current_line # Show where the comment is now in the diff
135-
is_effectively_outdated = True
136-
else: # Comment is current or a file-level comment (original_line is None but current_pos exists)
133+
# is_effectively_outdated is no longer needed with the new distinct flags
134+
135+
if current_pos is None:
136+
status_text = "[IRRELEVANT]"
137+
line_to_display = original_line
138+
elif original_line is not None and current_line != original_line:
139+
status_text = "[OLD]"
140+
line_to_display = current_line
141+
else:
137142
status_text = "[CURRENT]"
138-
line_to_display = current_line # For line comments, or None for file comments (handled by fallback)
139-
is_effectively_outdated = False
143+
line_to_display = current_line
140144

141145
if line_to_display is None:
142146
line_to_display = "N/A"
143147

144-
if args.skip_outdated and is_effectively_outdated:
148+
# New filtering logic
149+
if status_text == "[IRRELEVANT]" and not args.include_irrelevant:
150+
continue
151+
if status_text == "[OLD]" and args.exclude_old:
145152
continue
146153

147154
# Update latest timestamp (only for comments that will be printed)
@@ -159,6 +166,7 @@ def main():
159166
except ValueError:
160167
sys.stderr.write(f"Warning: Could not parse timestamp: {created_at_str}\n")
161168

169+
# Get other comment details
162170
user = comment.get("user", {}).get("login", "Unknown user")
163171
path = comment.get("path", "N/A")
164172
body = comment.get("body", "").strip()

0 commit comments

Comments
 (0)