Skip to content

Commit 81be829

Browse files
committed
Refactor GitHub comment generation to eliminate duplicate template strings
- Extract shared comment formatting logic into format_comment() function - Remove duplicate markdown template from get_comment_body() and get_summary_comment_body() - Improve code maintainability by centralizing template structure - Update test snapshots to reflect cleaner output (removed extra blank line) - Fix truncate_sql_if_needed() function signature to match actual usage
1 parent 5093f59 commit 81be829

4 files changed

+60
-54
lines changed

crates/squawk/src/github.rs

Lines changed: 57 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ pub fn check_and_comment_on_pr(
167167

168168
fn get_comment_body(files: &[CheckReport], version: &str) -> String {
169169
let violations_count: usize = files.iter().map(|x| x.violations.len()).sum();
170-
171170
let violations_emoji = get_violations_emoji(violations_count);
172171

173172
// First, try to generate the full comment
@@ -176,29 +175,19 @@ fn get_comment_body(files: &[CheckReport], version: &str) -> String {
176175
.filter_map(|x| get_sql_file_content(x).ok())
177176
.collect();
178177

179-
let full_comment = format!(
180-
r"
181-
{COMMENT_HEADER}
182-
183-
### **{violations_emoji} {violation_count}** violations across **{file_count}** file(s)
184-
185-
---
186-
{sql_file_content}
187-
188-
[📚 More info on rules](https://github.com/sbdchd/squawk#rules)
189-
190-
⚡️ Powered by [`Squawk`](https://github.com/sbdchd/squawk) ({version}), a linter for PostgreSQL, focused on migrations
191-
",
192-
violations_emoji = violations_emoji,
193-
violation_count = violations_count,
194-
file_count = files.len(),
195-
sql_file_content = sql_file_contents.join("\n"),
196-
version = version
178+
let content = sql_file_contents.join("\n");
179+
let full_comment = format_comment(
180+
violations_emoji,
181+
violations_count,
182+
files.len(),
183+
&content,
184+
version,
185+
None, // No summary notice for full comments
197186
);
198187

199188
// Check if the comment exceeds GitHub's size limit
200189
if full_comment.len() <= GITHUB_COMMENT_MAX_SIZE {
201-
return full_comment.trim_matches('\n').into();
190+
return full_comment;
202191
}
203192

204193
// If the comment is too large, create a summary instead
@@ -246,45 +235,72 @@ fn get_summary_comment_body(
246235
file_summaries.push(summary);
247236
}
248237

238+
let summary_notice = Some("⚠️ **Large Report**: This report was summarized due to size constraints. SQL content has been omitted but all violations were analyzed.");
239+
240+
format_comment(
241+
violations_emoji,
242+
violations_count,
243+
files.len(),
244+
&file_summaries.join("\n"),
245+
version,
246+
summary_notice,
247+
)
248+
}
249+
250+
const fn get_violations_emoji(count: usize) -> &'static str {
251+
if count > 0 { "🚒" } else { "✅" }
252+
}
253+
254+
fn format_comment(
255+
violations_emoji: &str,
256+
violation_count: usize,
257+
file_count: usize,
258+
content: &str,
259+
version: &str,
260+
summary_notice: Option<&str>,
261+
) -> String {
262+
let notice_section = if let Some(notice) = summary_notice {
263+
format!("\n> {}\n", notice)
264+
} else {
265+
String::new()
266+
};
267+
249268
format!(
250269
r"
251270
{COMMENT_HEADER}
252271
253-
### **{violations_emoji} {violation_count}** violations across **{file_count}** file(s)
254-
255-
> ⚠️ **Large Report**: This report was summarized due to size constraints. SQL content has been omitted but all violations were analyzed.
256-
272+
### **{violations_emoji} {violation_count}** violations across **{file_count}** file(s){notice_section}
257273
---
258-
{file_summaries}
274+
{content}
259275
260276
[📚 More info on rules](https://github.com/sbdchd/squawk#rules)
261277
262278
⚡️ Powered by [`Squawk`](https://github.com/sbdchd/squawk) ({version}), a linter for PostgreSQL, focused on migrations
263279
",
264280
violations_emoji = violations_emoji,
265-
violation_count = violations_count,
266-
file_count = files.len(),
267-
file_summaries = file_summaries.join("\n"),
281+
violation_count = violation_count,
282+
file_count = file_count,
283+
notice_section = notice_section,
284+
content = content,
268285
version = version
269286
)
270287
.trim_matches('\n')
271288
.into()
272289
}
273290

274-
const fn get_violations_emoji(count: usize) -> &'static str {
275-
if count > 0 { "🚒" } else { "✅" }
276-
}
277-
278-
fn truncate_sql_if_needed(sql: &str, max_lines: usize) -> (String, bool) {
291+
fn truncate_sql_if_needed(sql: &str) -> (String, bool) {
279292
let lines: Vec<&str> = sql.lines().collect();
280-
if lines.len() <= max_lines {
293+
if lines.len() <= MAX_SQL_PREVIEW_LINES {
281294
(sql.to_string(), false)
282295
} else {
283-
let truncated_lines = lines[..max_lines].join("\n");
284-
let remaining_lines = lines.len() - max_lines;
296+
let truncated_lines = lines[..MAX_SQL_PREVIEW_LINES].join("
297+
");
298+
let remaining_lines = lines.len() - MAX_SQL_PREVIEW_LINES;
285299
(
286300
format!(
287-
"{truncated_lines}\n\n-- ... ({remaining_lines} more lines truncated for brevity)"
301+
"{truncated_lines}
302+
303+
-- ... ({remaining_lines} more lines truncated for brevity)"
288304
),
289305
true,
290306
)
@@ -314,7 +330,7 @@ fn get_sql_file_content(violation: &CheckReport) -> Result<String> {
314330
};
315331

316332
let violations_emoji = get_violations_emoji(violation_count);
317-
let (display_sql, was_truncated) = truncate_sql_if_needed(sql, MAX_SQL_PREVIEW_LINES);
333+
let (display_sql, was_truncated) = truncate_sql_if_needed(sql);
318334

319335
let truncation_notice = if was_truncated {
320336
"\n\n> ⚠️ **Note**: SQL content has been truncated for display purposes. The full analysis was performed on the complete file."
@@ -432,17 +448,17 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
432448
}
433449

434450
#[test]
435-
fn test_sql_truncation() {
451+
fn sql_truncation() {
436452
let short_sql = "SELECT 1;";
437-
let (result, truncated) = crate::github::truncate_sql_if_needed(short_sql, 5);
453+
let (result, truncated) = crate::github::truncate_sql_if_needed(short_sql);
438454
assert!(!truncated);
439455
assert_eq!(result, short_sql);
440456

441457
let long_sql = (0..100)
442458
.map(|i| format!("-- Line {}", i))
443459
.collect::<Vec<_>>()
444460
.join("\n");
445-
let (result, truncated) = crate::github::truncate_sql_if_needed(&long_sql, 50);
461+
let (result, truncated) = crate::github::truncate_sql_if_needed(&long_sql);
446462
assert!(truncated);
447463
assert!(result.contains("-- ... (50 more lines truncated for brevity)"));
448464
}
@@ -508,13 +524,6 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
508524

509525
let body = get_comment_body(&violations, "0.2.3");
510526

511-
// Debug: Print the actual size to see what's happening
512-
println!(
513-
"Comment body size: {}, limit: {}",
514-
body.len(),
515-
super::GITHUB_COMMENT_MAX_SIZE
516-
);
517-
518527
// The comment should be within GitHub's size limits
519528
assert!(body.len() <= super::GITHUB_COMMENT_MAX_SIZE);
520529

crates/squawk/src/snapshots/squawk__github__test_github_comment__generating_comment_multiple_files.snap

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
---
2-
source: crates/cli/src/github.rs
2+
source: crates/squawk/src/github.rs
33
expression: body
44
---
55
# Squawk Report
66

77
### **🚒 1** violations across **1** file(s)
8-
98
---
109

1110
<h3><code>alpha.sql</code></h3>

crates/squawk/src/snapshots/squawk__github__test_github_comment__generating_comment_no_violations.snap

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
---
2-
source: crates/cli/src/github.rs
2+
source: crates/squawk/src/github.rs
33
expression: body
44
---
55
# Squawk Report
66

77
### **0** violations across **2** file(s)
8-
98
---
109

1110
<h3><code>alpha.sql</code></h3>

crates/squawk/src/snapshots/squawk__github__test_github_comment__generating_no_violations_no_files.snap

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
---
2-
source: crates/cli/src/github.rs
2+
source: crates/squawk/src/github.rs
33
expression: body
44
---
55
# Squawk Report
66

77
### **0** violations across **0** file(s)
8-
98
---
109

1110

0 commit comments

Comments
 (0)