Skip to content

Commit

Permalink
allow PR review with no suggestions in diff (#68)
Browse files Browse the repository at this point in the history
* allow PR review summary without suggestions
  resolves #67
  This is intended to let users reduce comments posted in PR reviews.
* add test condition
  • Loading branch information
2bndy5 committed Feb 11, 2024
1 parent 2cb6422 commit 5ad8d43
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 14 deletions.
19 changes: 14 additions & 5 deletions cpp_linter/rest_api/github_api.py
Expand Up @@ -376,18 +376,24 @@ def post_review(
body = f"{COMMENT_MARKER}## Cpp-linter Review\n"
payload_comments = []
total_changes = 0
summary_only = (
environ.get("CPP_LINTER_PR_REVIEW_SUMMARY_ONLY", "false") == "true"
)
advice: Dict[str, Sequence[Union[TidyAdvice, FormatAdvice]]] = {}
if format_review:
advice["clang-format"] = format_advice
if tidy_review:
advice["clang-tidy"] = tidy_advice
for tool_name, tool_advice in advice.items():
comments, total, patch = self.create_review_comments(files, tool_advice)
comments, total, patch = self.create_review_comments(
files, tool_advice, summary_only
)
total_changes += total
payload_comments.extend(comments)
if total and total != len(comments):
body += f"Only {len(comments)} out of {total} {tool_name} "
body += "suggestions fit within this pull request's diff.\n"
if not summary_only:
payload_comments.extend(comments)
if total and total != len(comments):
body += f"Only {len(comments)} out of {total} {tool_name} "
body += "suggestions fit within this pull request's diff.\n"
if patch:
body += f"\n<details><summary>Click here for the full {tool_name} patch"
body += f"</summary>\n\n\n```diff\n{patch}\n```\n\n\n</details>\n\n"
Expand All @@ -413,6 +419,7 @@ def post_review(
def create_review_comments(
files: List[FileObj],
tool_advice: Sequence[Union[FormatAdvice, TidyAdvice]],
summary_only: bool,
) -> Tuple[List[Dict[str, Any]], int, str]:
"""Creates a batch of comments for a specific clang tool's PR review"""
total = 0
Expand All @@ -430,6 +437,8 @@ def create_review_comments(
full_patch += patch.text
for hunk in patch.hunks:
total += 1
if summary_only:
continue
new_hunk_range = file.is_hunk_contained(hunk)
if new_hunk_range is None:
continue
Expand Down
23 changes: 14 additions & 9 deletions tests/reviews/test_pr_review.py
Expand Up @@ -13,18 +13,19 @@


@pytest.mark.parametrize(
"is_draft,is_closed,with_token,force_approved,tidy_review,format_review,changes",
"is_draft,is_closed,with_token,force_approved,tidy_review,format_review,changes,summary_only",
[
(True, False, True, False, False, True, 2),
(False, True, True, False, False, True, 2),
(True, False, True, False, False, True, 2, False),
(False, True, True, False, False, True, 2, False),
pytest.param(
False, False, False, False, False, True, 2, marks=pytest.mark.xfail
False, False, False, False, False, True, 2, False, marks=pytest.mark.xfail
),
(False, False, True, True, False, True, 2),
(False, False, True, False, True, False, 2),
(False, False, True, False, False, True, 2),
(False, False, True, False, True, True, 1),
(False, False, True, False, True, True, 0),
(False, False, True, True, False, True, 2, False),
(False, False, True, False, True, False, 2, False),
(False, False, True, False, False, True, 2, False),
(False, False, True, False, True, True, 1, False),
(False, False, True, False, True, True, 0, False),
(False, False, True, False, True, True, 0, True),
],
ids=[
"draft",
Expand All @@ -35,6 +36,7 @@
"format", # changes == diff_chunks only
"lines_added",
"all_lines",
"summary_only",
],
)
def test_post_review(
Expand All @@ -47,6 +49,7 @@ def test_post_review(
format_review: bool,
force_approved: bool,
changes: int,
summary_only: bool,
):
"""A mock test of posting PR reviews"""
# patch env vars
Expand All @@ -57,6 +60,8 @@ def test_post_review(
monkeypatch.setenv("CI", "true")
if with_token:
monkeypatch.setenv("GITHUB_TOKEN", "123456")
if summary_only:
monkeypatch.setenv("CPP_LINTER_PR_REVIEW_SUMMARY_ONLY", "true")
monkeypatch.chdir(str(tmp_path))
(tmp_path / "src").mkdir()
demo_dir = Path(__file__).parent.parent / "demo"
Expand Down

0 comments on commit 5ad8d43

Please sign in to comment.