Skip to content

Commit d15f11e

Browse files
Style: Final comment cleanup and text refinements
- Removes the unused `TIMEOUT_LONG` constant. - Adds a blank line after imports for PEP 8 compliance. - Changes the output heading for overall review bodies from "### Summary Comment:" to "### Comment:". - Performs a final pass to remove extraneous, obsolete, or iteration-related comments throughout the script, while retaining comments that explain non-obvious logic or important context.
1 parent 76fa16c commit d15f11e

File tree

1 file changed

+22
-37
lines changed

1 file changed

+22
-37
lines changed

scripts/print_github_reviews.py

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@
2626
import subprocess
2727
from requests.adapters import HTTPAdapter
2828
from requests.packages.urllib3.util.retry import Retry
29+
2930
# Constants for GitHub API interaction
3031
RETRIES = 3
3132
BACKOFF = 5
3233
RETRY_STATUS = (403, 500, 502, 504) # HTTP status codes to retry on
3334
TIMEOUT = 5 # Default timeout for requests in seconds
34-
TIMEOUT_LONG = 20 # Longer timeout, currently not used by functions in this script
3535

3636
# Global variables for the target repository, populated by set_repo_url_standalone()
3737
OWNER = ''
@@ -374,7 +374,8 @@ def parse_repo_url(url_string):
374374
filtered_overall_reviews = []
375375
if overall_reviews: # If not None and not empty
376376
for review in overall_reviews:
377-
if review.get("state") == "DISMISSED":
377+
review_state = review.get("state")
378+
if review_state == "DISMISSED" or review_state == "PENDING":
378379
continue
379380

380381
if args.since:
@@ -404,31 +405,26 @@ def parse_repo_url(url_string):
404405
sys.stderr.write(f"Warning: Could not parse review submitted_at timestamp '{submitted_at_str}' or --since timestamp '{args.since}': {ve}\n")
405406
# If parsing fails, we might choose to include the review to be safe, or skip. Current: include.
406407

407-
# New filter: Exclude "COMMENTED" reviews with an empty body
408408
if review.get("state") == "COMMENTED" and not review.get("body", "").strip():
409409
continue
410410

411411
filtered_overall_reviews.append(review)
412412

413-
# Sort by submission time, oldest first
414413
try:
415414
filtered_overall_reviews.sort(key=lambda r: r.get("submitted_at", ""))
416415
except Exception as e: # Broad exception for safety
417416
sys.stderr.write(f"Warning: Could not sort overall reviews: {e}\n")
418417

419-
# Output overall reviews if any exist after filtering
420418
if filtered_overall_reviews:
421-
print("# Code Reviews\n\n") # Changed heading
422-
# Explicitly re-initialize before this loop to be absolutely sure for the update logic within it.
423-
# The main initialization at the top of main() handles the case where this block is skipped.
424-
temp_latest_overall_review_dt = None # Use a temporary variable for this loop's accumulation
419+
print("# Code Reviews\n\n")
420+
# Use a temporary variable for accumulating latest timestamp within this specific block
421+
temp_latest_overall_review_dt = None
425422
for review in filtered_overall_reviews:
426423
user = review.get("user", {}).get("login", "Unknown user")
427-
submitted_at_str = review.get("submitted_at", "N/A") # Keep original string for printing
424+
submitted_at_str = review.get("submitted_at", "N/A")
428425
state = review.get("state", "N/A")
429426
body = review.get("body", "").strip()
430427

431-
# Track latest overall review timestamp within this block
432428
if submitted_at_str and submitted_at_str != "N/A":
433429
try:
434430
if sys.version_info < (3, 11):
@@ -445,32 +441,29 @@ def parse_repo_url(url_string):
445441
review_id = review.get("id", "N/A")
446442

447443
print(f"## Review by: **{user}** (ID: `{review_id}`)\n")
448-
print(f"* **Submitted At**: `{submitted_at_str}`") # Print original string
444+
print(f"* **Submitted At**: `{submitted_at_str}`")
449445
print(f"* **State**: `{state}`")
450446
print(f"* **URL**: <{html_url}>\n")
451-
# Removed duplicated lines here
452447

453448
if body:
454-
print("\n### Summary Comment:")
449+
print("\n### Comment:") # Changed heading
455450
print(body)
456451
print("\n---")
457452

458453
# After processing all overall reviews in this block, update the main variable
459454
if temp_latest_overall_review_dt:
460455
latest_overall_review_activity_dt = temp_latest_overall_review_dt
461-
462-
# Add an extra newline to separate from line comments if any overall reviews were printed
463456
print("\n")
464457

465458

466459
sys.stderr.write(f"Fetching line comments for PR #{pull_request_number} from {OWNER}/{REPO}...\n")
467460
if args.since:
468-
sys.stderr.write(f"Filtering line comments updated since: {args.since}\n") # Clarify this 'since' is for line comments
461+
sys.stderr.write(f"Filtering line comments updated since: {args.since}\n")
469462

470463
comments = get_pull_request_review_comments(
471464
args.token,
472465
pull_request_number,
473-
since=args.since # This 'since' applies to line comment's 'updated_at'
466+
since=args.since
474467
)
475468

476469
if comments is None:
@@ -479,22 +472,15 @@ def parse_repo_url(url_string):
479472
# Note: The decision to exit if only line comments fail vs. if only overall reviews fail could be nuanced.
480473
# For now, failure to fetch either is treated as a critical error for the script's purpose.
481474

482-
# Initializations for latest_overall_review_activity_dt, latest_line_comment_activity_dt,
483-
# and processed_comments_count have been moved to the top of the main() function.
484-
485475
# Handling for line comments
486-
if not comments: # comments is an empty list here (None case handled above)
476+
if not comments:
487477
sys.stderr.write(f"No line comments found for PR #{pull_request_number} (or matching filters).\n")
488-
# If there were also no overall reviews, and no line comments, then nothing to show.
489-
# The 'next command' suggestion logic below will still run if overall_reviews had content.
490-
if not filtered_overall_reviews : # and not comments (implicitly true here)
491-
# Only return (and skip 'next command' suggestion) if NO content at all was printed.
492-
# If overall_reviews were printed, we still want the 'next command' suggestion.
493-
pass # Let it fall through to the 'next command' suggestion logic
478+
# If filtered_overall_reviews is also empty, then overall_latest_activity_dt will be None,
479+
# and no 'next command' suggestion will be printed. This is correct.
494480
else:
495481
print("# Review Comments\n\n")
496482

497-
for comment in comments: # if comments is empty, this loop is skipped.
483+
for comment in comments:
498484
created_at_str = comment.get("created_at")
499485

500486
current_pos = comment.get("position")
@@ -531,16 +517,16 @@ def parse_repo_url(url_string):
531517
else:
532518
dt_str_updated = updated_at_str
533519
current_comment_activity_dt = datetime.datetime.fromisoformat(dt_str_updated)
534-
if latest_line_comment_activity_dt is None or current_comment_activity_dt > latest_line_comment_activity_dt: # Corrected variable name
535-
latest_line_comment_activity_dt = current_comment_activity_dt # Corrected variable name
520+
if latest_line_comment_activity_dt is None or current_comment_activity_dt > latest_line_comment_activity_dt:
521+
latest_line_comment_activity_dt = current_comment_activity_dt
536522
except ValueError:
537523
sys.stderr.write(f"Warning: Could not parse line comment updated_at for --since suggestion: {updated_at_str}\n")
538524

539525
user = comment.get("user", {}).get("login", "Unknown user")
540526
path = comment.get("path", "N/A")
541527
body = comment.get("body", "").strip()
542528

543-
if not body: # Skip comments with no actual text content
529+
if not body:
544530
continue
545531

546532
processed_comments_count += 1
@@ -561,11 +547,11 @@ def parse_repo_url(url_string):
561547
print("\n### Context:")
562548
print("```")
563549
if diff_hunk and diff_hunk.strip():
564-
if args.context_lines == 0: # 0 means full hunk
550+
if args.context_lines == 0:
565551
print(diff_hunk)
566-
else: # Display header (if any) and last N lines
552+
else:
567553
hunk_lines = diff_hunk.split('\n')
568-
if hunk_lines and hunk_lines[0].startswith("@@ "): # Diff hunk header
554+
if hunk_lines and hunk_lines[0].startswith("@@ "):
569555
print(hunk_lines[0])
570556
hunk_lines = hunk_lines[1:]
571557
print("\n".join(hunk_lines[-args.context_lines:]))
@@ -590,14 +576,13 @@ def parse_repo_url(url_string):
590576

591577
if overall_latest_activity_dt:
592578
try:
593-
# Suggest next command with '--since' pointing to just after the latest activity.
594579
next_since_dt = overall_latest_activity_dt.astimezone(timezone.utc) + timedelta(seconds=2)
595580
next_since_str = next_since_dt.strftime('%Y-%m-%dT%H:%M:%SZ')
596581

597582
new_cmd_args = [sys.executable, sys.argv[0]]
598583
i = 1
599584
while i < len(sys.argv):
600-
if sys.argv[i] == "--since": # Skip existing --since argument and its value
585+
if sys.argv[i] == "--since":
601586
i += 2
602587
continue
603588
new_cmd_args.append(sys.argv[i])

0 commit comments

Comments
 (0)