Skip to content

Commit 9312a0c

Browse files
fix: Align 'since' filter and next command with observed API behavior (updated_at)
This commit modifies `scripts/gha/get_pr_review_comments.py` to correctly use `updated_at` timestamps for its `--since` filtering and the "suggest next command" feature. This aligns with the observed behavior of the GitHub API endpoint for listing pull request review comments, where the `since` parameter filters by update time rather than creation time (contrary to some initial documentation interpretations for this specific endpoint). Changes include: - The "suggest next command" feature now tracks the maximum `updated_at` timestamp from processed comments to calculate the `--since` value for the next suggested command. - The help text for the `--since` argument has been updated to clarify it filters by "updated at or after". - The informational message printed to stderr when the `--since` filter is active now also states "updated since". - The `created_at` timestamp continues to be displayed for each comment for informational purposes.
1 parent 94417e7 commit 9312a0c

File tree

1 file changed

+17
-16
lines changed

1 file changed

+17
-16
lines changed

scripts/gha/get_pr_review_comments.py

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ def main():
7777
"--since",
7878
type=str,
7979
default=None,
80-
help="Only show comments created at or after this ISO 8601 timestamp (e.g., YYYY-MM-DDTHH:MM:SSZ)."
80+
help="Only show comments updated at or after this ISO 8601 timestamp (e.g., YYYY-MM-DDTHH:MM:SSZ)."
8181
)
8282
parser.add_argument(
8383
"--exclude-old",
@@ -107,7 +107,7 @@ def main():
107107

108108
sys.stderr.write(f"Fetching comments for PR #{args.pull_number} from {firebase_github.OWNER}/{firebase_github.REPO}...\n")
109109
if args.since:
110-
sys.stderr.write(f"Filtering comments created since: {args.since}\n")
110+
sys.stderr.write(f"Filtering comments updated since: {args.since}\n")
111111
# Removed skip_outdated message block
112112

113113

@@ -121,7 +121,7 @@ def main():
121121
sys.stderr.write(f"No review comments found for PR #{args.pull_number} (or matching filters), or an error occurred.\n")
122122
return
123123

124-
latest_created_at_obj = None
124+
latest_activity_timestamp_obj = None
125125
processed_comments_count = 0
126126
print("# Review Comments\n\n")
127127
for comment in comments:
@@ -154,22 +154,23 @@ def main():
154154
if status_text == STATUS_OLD and args.exclude_old:
155155
continue
156156

157-
# Update latest timestamp (only for comments that will be printed)
158-
if created_at_str:
157+
# Update latest activity timestamp (only for comments that will be printed)
158+
# This will be based on updated_at for suggesting the next --since value.
159+
# created_at_str is still used for display.
160+
updated_at_str = comment.get("updated_at")
161+
if updated_at_str: # Check if updated_at_str is not None and not empty
159162
try:
160-
# GitHub ISO format "YYYY-MM-DDTHH:MM:SSZ"
161-
# Python <3.11 fromisoformat needs "+00:00" not "Z"
162163
if sys.version_info < (3, 11):
163-
dt_str = created_at_str.replace("Z", "+00:00")
164+
dt_str_updated = updated_at_str.replace("Z", "+00:00")
164165
else:
165-
dt_str = created_at_str
166-
current_comment_dt = datetime.datetime.fromisoformat(dt_str)
167-
if latest_created_at_obj is None or current_comment_dt > latest_created_at_obj:
168-
latest_created_at_obj = current_comment_dt
166+
dt_str_updated = updated_at_str
167+
current_comment_activity_dt = datetime.datetime.fromisoformat(dt_str_updated)
168+
if latest_activity_timestamp_obj is None or current_comment_activity_dt > latest_activity_timestamp_obj:
169+
latest_activity_timestamp_obj = current_comment_activity_dt
169170
except ValueError:
170-
sys.stderr.write(f"Warning: Could not parse timestamp: {created_at_str}\n")
171+
sys.stderr.write(f"Warning: Could not parse updated_at timestamp: {updated_at_str}\n")
171172

172-
# Get other comment details
173+
# Get other comment details (user is already fetched if needed for other logic)
173174
user = comment.get("user", {}).get("login", "Unknown user")
174175
path = comment.get("path", "N/A")
175176
body = comment.get("body", "").strip()
@@ -219,10 +220,10 @@ def main():
219220

220221
sys.stderr.write(f"\nPrinted {processed_comments_count} comments to stdout.\n")
221222

222-
if latest_created_at_obj:
223+
if latest_activity_timestamp_obj:
223224
try:
224225
# Ensure it's UTC before adding timedelta, then format
225-
next_since_dt = latest_created_at_obj.astimezone(timezone.utc) + timedelta(seconds=2)
226+
next_since_dt = latest_activity_timestamp_obj.astimezone(timezone.utc) + timedelta(seconds=2)
226227
next_since_str = next_since_dt.strftime('%Y-%m-%dT%H:%M:%SZ')
227228

228229
new_cmd_args = [sys.executable, sys.argv[0]] # Start with interpreter and script path

0 commit comments

Comments
 (0)