Skip to content

Commit 757daf2

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 16723cf commit 757daf2

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
@@ -160,7 +160,6 @@ pub fn check_and_comment_on_pr(
160160

161161
fn get_comment_body(files: &[CheckReport], version: &str) -> String {
162162
let violations_count: usize = files.iter().map(|x| x.violations.len()).sum();
163-
164163
let violations_emoji = get_violations_emoji(violations_count);
165164

166165
// First, try to generate the full comment
@@ -169,29 +168,19 @@ fn get_comment_body(files: &[CheckReport], version: &str) -> String {
169168
.filter_map(|x| get_sql_file_content(x).ok())
170169
.collect();
171170

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

192181
// Check if the comment exceeds GitHub's size limit
193182
if full_comment.len() <= GITHUB_COMMENT_MAX_SIZE {
194-
return full_comment.trim_matches('\n').into();
183+
return full_comment;
195184
}
196185

197186
// If the comment is too large, create a summary instead
@@ -239,45 +228,72 @@ fn get_summary_comment_body(
239228
file_summaries.push(summary);
240229
}
241230

231+
let summary_notice = Some("⚠️ **Large Report**: This report was summarized due to size constraints. SQL content has been omitted but all violations were analyzed.");
232+
233+
format_comment(
234+
violations_emoji,
235+
violations_count,
236+
files.len(),
237+
&file_summaries.join("\n"),
238+
version,
239+
summary_notice,
240+
)
241+
}
242+
243+
const fn get_violations_emoji(count: usize) -> &'static str {
244+
if count > 0 { "🚒" } else { "✅" }
245+
}
246+
247+
fn format_comment(
248+
violations_emoji: &str,
249+
violation_count: usize,
250+
file_count: usize,
251+
content: &str,
252+
version: &str,
253+
summary_notice: Option<&str>,
254+
) -> String {
255+
let notice_section = if let Some(notice) = summary_notice {
256+
format!("\n> {}\n", notice)
257+
} else {
258+
String::new()
259+
};
260+
242261
format!(
243262
r"
244263
{COMMENT_HEADER}
245264
246-
### **{violations_emoji} {violation_count}** violations across **{file_count}** file(s)
247-
248-
> ⚠️ **Large Report**: This report was summarized due to size constraints. SQL content has been omitted but all violations were analyzed.
249-
265+
### **{violations_emoji} {violation_count}** violations across **{file_count}** file(s){notice_section}
250266
---
251-
{file_summaries}
267+
{content}
252268
253269
[📚 More info on rules](https://github.com/sbdchd/squawk#rules)
254270
255271
⚡️ Powered by [`Squawk`](https://github.com/sbdchd/squawk) ({version}), a linter for PostgreSQL, focused on migrations
256272
",
257273
violations_emoji = violations_emoji,
258-
violation_count = violations_count,
259-
file_count = files.len(),
260-
file_summaries = file_summaries.join("\n"),
274+
violation_count = violation_count,
275+
file_count = file_count,
276+
notice_section = notice_section,
277+
content = content,
261278
version = version
262279
)
263280
.trim_matches('\n')
264281
.into()
265282
}
266283

267-
const fn get_violations_emoji(count: usize) -> &'static str {
268-
if count > 0 { "🚒" } else { "✅" }
269-
}
270-
271-
fn truncate_sql_if_needed(sql: &str, max_lines: usize) -> (String, bool) {
284+
fn truncate_sql_if_needed(sql: &str) -> (String, bool) {
272285
let lines: Vec<&str> = sql.lines().collect();
273-
if lines.len() <= max_lines {
286+
if lines.len() <= MAX_SQL_PREVIEW_LINES {
274287
(sql.to_string(), false)
275288
} else {
276-
let truncated_lines = lines[..max_lines].join("\n");
277-
let remaining_lines = lines.len() - max_lines;
289+
let truncated_lines = lines[..MAX_SQL_PREVIEW_LINES].join("
290+
");
291+
let remaining_lines = lines.len() - MAX_SQL_PREVIEW_LINES;
278292
(
279293
format!(
280-
"{truncated_lines}\n\n-- ... ({remaining_lines} more lines truncated for brevity)"
294+
"{truncated_lines}
295+
296+
-- ... ({remaining_lines} more lines truncated for brevity)"
281297
),
282298
true,
283299
)
@@ -307,7 +323,7 @@ fn get_sql_file_content(violation: &CheckReport) -> Result<String> {
307323
};
308324

309325
let violations_emoji = get_violations_emoji(violation_count);
310-
let (display_sql, was_truncated) = truncate_sql_if_needed(sql, MAX_SQL_PREVIEW_LINES);
326+
let (display_sql, was_truncated) = truncate_sql_if_needed(sql);
311327

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

427443
#[test]
428-
fn test_sql_truncation() {
444+
fn sql_truncation() {
429445
let short_sql = "SELECT 1;";
430-
let (result, truncated) = crate::github::truncate_sql_if_needed(short_sql, 5);
446+
let (result, truncated) = crate::github::truncate_sql_if_needed(short_sql);
431447
assert!(!truncated);
432448
assert_eq!(result, short_sql);
433449

434450
let long_sql = (0..100)
435451
.map(|i| format!("-- Line {}", i))
436452
.collect::<Vec<_>>()
437453
.join("\n");
438-
let (result, truncated) = crate::github::truncate_sql_if_needed(&long_sql, 50);
454+
let (result, truncated) = crate::github::truncate_sql_if_needed(&long_sql);
439455
assert!(truncated);
440456
assert!(result.contains("-- ... (50 more lines truncated for brevity)"));
441457
}
@@ -501,13 +517,6 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
501517

502518
let body = get_comment_body(&violations, "0.2.3");
503519

504-
// Debug: Print the actual size to see what's happening
505-
println!(
506-
"Comment body size: {}, limit: {}",
507-
body.len(),
508-
super::GITHUB_COMMENT_MAX_SIZE
509-
);
510-
511520
// The comment should be within GitHub's size limits
512521
assert!(body.len() <= super::GITHUB_COMMENT_MAX_SIZE);
513522

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)