Skip to content

Commit e208a98

Browse files
authored
Handle PR reviews. (#134)
The main comment on a review wasn't being handled, and this change interleaves it with the PR comments. GitHub treats these as different entities. I'm hiding reviews with no top-level comment; these didn't seem worth including empty comments. Links are being added to PR comments to make them easier to find. This means indenting the content, so now all comments (PR or review thread) are consistently indented, removing a parameter. Also, I'm adjusting the formatting of review threads to match. snippets for comparison: ``` #83 (comment) googlebot: All (the pull request submitter and all commit authors) CLAs are... #83 (review) chandlerc: Overall, I think this is a pretty significant improved direction... ``` and from a review thread: ``` https://github.com/carbon-language/carbon-lang/pull/83/files/9863da8#diff-8f28081abb21e65fba5a0b0b29404c78R16 - line 16; unresolved - diff: https://github.com/carbon-language/carbon-lang/pull/83/files/9863da8..HEAD#diff-8f28081abb21e65fba5a0b0b29404c78L16 josh11b: ```suggestion¶ - Block comments begin with a line starting with `/... jonmeow: "begin with a line starting with" phrasing is a little confusing t... josh11b: As I understand @zygoloid 's lexing proposal, there may be additio... jonmeow: Rewritten with example ```
1 parent 388b2de commit e208a98

File tree

2 files changed

+188
-98
lines changed

2 files changed

+188
-98
lines changed

src/scripts/pr_comments.py

Lines changed: 107 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@
2121
author {
2222
login
2323
}
24+
createdAt
2425
title
2526
2627
%(comments)s
28+
%(reviews)s
2729
%(review_threads)s
2830
}
2931
}
@@ -47,6 +49,23 @@
4749
}
4850
"""
4951

52+
_QUERY_REVIEWS = """
53+
reviews(first: 100%s) {
54+
nodes {
55+
author {
56+
login
57+
}
58+
body
59+
createdAt
60+
url
61+
}
62+
pageInfo {
63+
endCursor
64+
hasNextPage
65+
}
66+
}
67+
"""
68+
5069
_QUERY_REVIEW_THREADS = """
5170
reviewThreads(first: 100%s) {
5271
nodes {
@@ -98,8 +117,8 @@ def from_raw_comment(raw_comment):
98117
)
99118

100119
@staticmethod
101-
def _rewrap(content, indent):
102-
"""Rewraps a comment to fit in 80 columns with an optional indent."""
120+
def _rewrap(content):
121+
"""Rewraps a comment to fit in 80 columns with an indent."""
103122
lines = []
104123
for line in content.split("\n"):
105124
lines.extend(
@@ -108,31 +127,49 @@ def _rewrap(content, indent):
108127
for x in textwrap.wrap(
109128
line,
110129
width=80,
111-
initial_indent=" " * indent,
112-
subsequent_indent=" " * indent,
130+
initial_indent=" " * 4,
131+
subsequent_indent=" " * 4,
113132
)
114133
]
115134
)
116135
return "\n".join(lines)
117136

118-
def format(self, long, indent):
137+
def format(self, long):
119138
"""Formats the comment."""
120139
if long:
121140
return "%s%s at %s:\n%s" % (
122-
" " * indent,
141+
" " * 2,
123142
self.author,
124143
self.timestamp.strftime("%Y-%m-%d %H:%M"),
125-
self._rewrap(self.body, indent + 2),
144+
self._rewrap(self.body),
126145
)
127146
else:
128147
# Compact newlines down into pilcrows, leaving a space after.
129148
body = self.body.replace("\r", "").replace("\n", "¶ ")
130149
while "¶ ¶" in body:
131150
body = body.replace("¶ ¶", "¶¶")
132-
line = "%s%s: %s" % (" " * indent, self.author, body)
151+
line = "%s%s: %s" % (" " * 2, self.author, body)
133152
return line if len(line) <= 80 else line[:77] + "..."
134153

135154

155+
class _PRComment(_Comment):
156+
"""A comment on the top-level PR."""
157+
158+
def __init__(self, raw_comment):
159+
super().__init__(
160+
raw_comment["author"]["login"],
161+
raw_comment["createdAt"],
162+
raw_comment["body"],
163+
)
164+
self.url = raw_comment["url"]
165+
166+
def __lt__(self, other):
167+
return self.timestamp < other.timestamp
168+
169+
def format(self, long):
170+
return "%s\n%s" % (self.url, super().format(long))
171+
172+
136173
class _Thread(object):
137174
"""A review thread on a line of code."""
138175

@@ -144,36 +181,29 @@ def __init__(self, parsed_args, thread):
144181
self.line = first_comment["originalPosition"]
145182
self.path = first_comment["path"]
146183

147-
# Link to the comment in the commit if possible; GitHub features work
148-
# better there than in the conversation view. The diff_url allows
149-
# viewing changes since the comment, although the comment won't be
150-
# visible there.
151-
if "originalCommit" in first_comment:
152-
template = (
153-
"https://github.com/carbon-language/%(repo)s/pull/%(pr_num)s/"
154-
"files/%(oid)s%(head)s#diff-%(path_md5)s%(line_side)s%(line)s"
155-
)
156-
# GitHub uses an md5 of the file's path for the link.
157-
path_md5 = hashlib.md5()
158-
path_md5.update(bytearray(self.path, "utf-8"))
159-
format_dict = {
160-
"head": "",
161-
"line_side": "R",
162-
"line": self.line,
163-
"oid": first_comment["originalCommit"]["abbreviatedOid"],
164-
"path_md5": path_md5.hexdigest(),
165-
"pr_num": parsed_args.pr_num,
166-
"repo": parsed_args.repo,
167-
}
168-
self.comment_url = template % format_dict
169-
format_dict["head"] = "..HEAD"
170-
format_dict["line_side"] = "L"
171-
self.diff_url = template % format_dict
172-
self.conversation_url = None
173-
else:
174-
self.conversation_url = first_comment["url"]
175-
self.comment_url = None
176-
self.diff_url = None
184+
# Link to the comment in the commit; GitHub features work better there
185+
# than in the conversation view. The diff_url allows viewing changes
186+
# since the comment, although the comment won't be visible there.
187+
template = (
188+
"https://github.com/carbon-language/%(repo)s/pull/%(pr_num)s/"
189+
"files/%(oid)s%(head)s#diff-%(path_md5)s%(line_side)s%(line)s"
190+
)
191+
# GitHub uses an md5 of the file's path for the link.
192+
path_md5 = hashlib.md5()
193+
path_md5.update(bytearray(self.path, "utf-8"))
194+
format_dict = {
195+
"head": "",
196+
"line_side": "R",
197+
"line": self.line,
198+
"oid": first_comment["originalCommit"]["abbreviatedOid"],
199+
"path_md5": path_md5.hexdigest(),
200+
"pr_num": parsed_args.pr_num,
201+
"repo": parsed_args.repo,
202+
}
203+
self.url = template % format_dict
204+
format_dict["head"] = "..HEAD"
205+
format_dict["line_side"] = "L"
206+
self.diff_url = template % format_dict
177207

178208
self.comments = [
179209
_Comment.from_raw_comment(comment)
@@ -198,16 +228,17 @@ def format(self, long):
198228
"""Formats the review thread with comments."""
199229
lines = []
200230
lines.append(
201-
"line %d; %s"
202-
% (self.line, ("resolved" if self.is_resolved else "unresolved"),)
231+
"%s\n - line %d; %s"
232+
% (
233+
self.url,
234+
self.line,
235+
("resolved" if self.is_resolved else "unresolved"),
236+
)
203237
)
204238
if self.diff_url:
205-
lines.append(" COMMENT: %s" % self.comment_url)
206-
lines.append(" CHANGES: %s" % self.diff_url)
207-
else:
208-
lines.append(" %s" % self.conversation_url)
239+
lines.append(" - diff: %s" % self.diff_url)
209240
for comment in self.comments:
210-
lines.append(comment.format(long, 2))
241+
lines.append(comment.format(long))
211242
return "\n".join(lines)
212243

213244
def has_comment_from(self, comments_from):
@@ -285,6 +316,7 @@ def _query(parsed_args, client, field_name=None, field=None):
285316
"repo": parsed_args.repo,
286317
"comments": "",
287318
"review_threads": "",
319+
"reviews": "",
288320
}
289321
if field:
290322
# Use a cursor for pagination of the field.
@@ -293,19 +325,25 @@ def _query(parsed_args, client, field_name=None, field=None):
293325
format_inputs["comments"] = _QUERY_COMMENTS % cursor
294326
elif field_name == "reviewThreads":
295327
format_inputs["review_threads"] = _QUERY_REVIEW_THREADS % cursor
328+
elif field_name == "reviews":
329+
format_inputs["reviews"] = _QUERY_REVIEWS % cursor
296330
else:
297331
raise ValueError("Unexpected field_name: %s" % field_name)
298332
else:
299-
# Fetch the first page of both fields.
333+
# Fetch the first page of all fields.
300334
format_inputs["comments"] = _QUERY_COMMENTS % ""
301335
format_inputs["review_threads"] = _QUERY_REVIEW_THREADS % ""
336+
format_inputs["reviews"] = _QUERY_REVIEWS % ""
302337
return client.execute(gql.gql(_QUERY % format_inputs))
303338

304339

305-
def _accumulate_comments(parsed_args, comments, raw_comments):
306-
"""Collects top-level comments."""
340+
def _accumulate_pr_comments(parsed_args, comments, raw_comments):
341+
"""Collects top-level comments and reviews."""
307342
for raw_comment in raw_comments:
308-
comments.append(_Comment.from_raw_comment(raw_comment))
343+
# Elide reviews that have no top-level comment body.
344+
if not raw_comment["body"]:
345+
continue
346+
comments.append(_PRComment(raw_comment))
309347

310348

311349
def _accumulate_threads(parsed_args, threads_by_path, raw_threads):
@@ -373,11 +411,20 @@ def _fetch_comments(parsed_args):
373411
threads_result = _query(parsed_args, client)
374412
pull_request = threads_result["repository"]["pullRequest"]
375413

376-
# Paginate comments and review threads.
414+
# Paginate comments, reviews, and review threads.
377415
comments = []
378416
_paginate(
379417
"comments",
380-
_accumulate_comments,
418+
_accumulate_pr_comments,
419+
parsed_args,
420+
client,
421+
pull_request,
422+
comments,
423+
)
424+
# Combine reviews into comments for interleaving.
425+
_paginate(
426+
"reviews",
427+
_accumulate_pr_comments,
381428
parsed_args,
382429
client,
383430
pull_request,
@@ -394,20 +441,23 @@ def _fetch_comments(parsed_args):
394441
)
395442

396443
# Now that loading is done (no more progress indicators), print the header.
397-
print(
398-
"\n '%s' by %s"
399-
% (pull_request["title"], pull_request["author"]["login"])
444+
print()
445+
pr_desc = _Comment(
446+
pull_request["author"]["login"],
447+
pull_request["createdAt"],
448+
pull_request["title"],
400449
)
450+
print(pr_desc.format(parsed_args.long))
401451
return comments, threads_by_path
402452

403453

404454
def main():
405455
parsed_args = _parse_args()
406456
comments, threads_by_path = _fetch_comments(parsed_args)
407457

408-
print()
409-
for comment in comments:
410-
print(comment.format(parsed_args.long, 0))
458+
for comment in sorted(comments):
459+
print()
460+
print(comment.format(parsed_args.long))
411461

412462
for path, threads in sorted(threads_by_path.items()):
413463
# Print a header for each path.

0 commit comments

Comments
 (0)