Skip to content

Commit 959ae38

Browse files
authored
feat: add logs about summarizing PR reviews (#371)
resolves #341
1 parent 619e877 commit 959ae38

1 file changed

Lines changed: 20 additions & 5 deletions

File tree

  • cpp-linter/src/clang_tools

cpp-linter/src/clang_tools/mod.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,10 @@ impl ReviewComments {
266266
);
267267
}
268268
if !summary_only && total_review_comments != self.tool_total {
269+
log::info!(
270+
"Only {total_review_comments} out of {} concerns fit within this pull request's diff.",
271+
self.tool_total
272+
);
269273
body.push_str(
270274
format!(
271275
"\nOnly {total_review_comments} out of {} concerns fit within this pull request's diff.\n",
@@ -276,12 +280,12 @@ impl ReviewComments {
276280
}
277281
// total number of comments can only go down after culling comments found in previous reviews.
278282
if total_review_comments > total {
283+
let dupes = total_review_comments - total;
284+
log::info!(
285+
"Found and removed {dupes} concerns that were duplicates of previous reviews."
286+
);
279287
body.push_str(
280-
format!(
281-
"\n{} suggestions were duplicates of previous reviews.\n",
282-
total_review_comments - total,
283-
)
284-
.as_str(),
288+
format!("\n{dupes} suggestions were duplicates of previous reviews.\n").as_str(),
285289
);
286290
}
287291
// The `full_patch` includes all suggestions that didn't fit in the diff.
@@ -313,6 +317,7 @@ impl ReviewComments {
313317
}
314318
} else if total_review_comments == 0 {
315319
// Only congratulate if there was no reused comments
320+
log::info!("No concerns to report: LGTM");
316321
body.push_str("\nNo concerns to report. Great job! :tada:\n");
317322
}
318323
body.push_str(USER_OUTREACH);
@@ -391,6 +396,11 @@ mod tests {
391396

392397
const PSEUDO_VERSION: Version = Version::new(15, 0, 0);
393398

399+
/// This test simulates removed suggestions that were reused in other PR reviews.
400+
///
401+
/// We do this as a arbitrary unit test because different clang tools versions
402+
/// produce different suggestions, which makes any attempt in integrations tests
403+
/// rather non-deterministic.
394404
#[test]
395405
fn summarize_reused_reviews() {
396406
let comments = vec![ReviewComment {
@@ -405,6 +415,11 @@ mod tests {
405415
};
406416
let total_review_comments = 2;
407417
let summary_only = false;
418+
#[cfg(feature = "bin")]
419+
{
420+
crate::logger::try_init();
421+
log::set_max_level(log::LevelFilter::Info);
422+
}
408423
let review_summary = ReviewComments::default().summarize(
409424
&clang_versions,
410425
&comments,

0 commit comments

Comments
 (0)