Skip to content

Commit 5429b2e

Browse files
alandefreitasalandefreitas
committed
ci(danger): align report table and add top-files summary
1 parent 76b7b43 commit 5429b2e

File tree

4 files changed

+181
-88
lines changed

4 files changed

+181
-88
lines changed

‎util/danger/dangerfile.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import { runDanger } from "./runner";
1111

1212
// Provided globally by Danger at runtime; declared here for editors/typecheckers.
13-
declare function warn(message: string, file?: string, line?: number): void;
13+
declare function markdown(message: string, file?: string, line?: number): void;
1414

1515
/**
1616
* Entrypoint for Danger; delegates to the rule runner.
@@ -20,6 +20,6 @@ export default async function dangerfile(): Promise<void> {
2020
try {
2121
await runDanger();
2222
} catch (error) {
23-
warn(`Danger checks hit an unexpected error: ${String(error)}`);
23+
markdown(["> [!WARNING]", `> Danger checks hit an unexpected error: ${String(error)}`].join("\n"));
2424
}
2525
}

‎util/danger/logic.ts‎

Lines changed: 168 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,30 @@ export interface ScopeTotals {
5959
files: number;
6060
additions: number;
6161
deletions: number;
62+
status: { added: number; modified: number; removed: number; renamed: number; other: number };
63+
}
64+
65+
export interface FileSummary {
66+
filename: string;
67+
scope: ScopeKey;
68+
additions: number;
69+
deletions: number;
70+
churn: number;
71+
status?: string;
6272
}
6373

6474
/**
6575
* Scope summary output that is rendered into a Markdown table.
6676
*/
6777
export interface ScopeReport {
6878
totals: Record<ScopeKey, ScopeTotals>;
69-
overall: { files: number; additions: number; deletions: number };
79+
overall: {
80+
files: number;
81+
additions: number;
82+
deletions: number;
83+
status: { added: number; modified: number; removed: number; renamed: number; other: number };
84+
};
85+
topFiles: FileSummary[];
7086
markdown: string;
7187
highlights: string[];
7288
}
@@ -90,6 +106,19 @@ export interface DangerResult {
90106
summary: ScopeReport;
91107
}
92108

109+
/** Display order for scopes in rendered reports. */
110+
export const scopeDisplayOrder: ScopeKey[] = [
111+
"source",
112+
"tests",
113+
"golden-tests",
114+
"docs",
115+
"ci",
116+
"build",
117+
"tooling",
118+
"third-party",
119+
"other",
120+
];
121+
93122
const allowedTypes = [
94123
"feat",
95124
"fix",
@@ -110,6 +139,33 @@ const skipTestLabels = new Set(["no-tests-needed", "skip-tests", "tests-not-requ
110139
const skipTestMarkers = ["[skip danger tests]", "[danger skip tests]"];
111140
const nonTestCommitLimit = 800;
112141

142+
/**
143+
* Format churn as a + / - pair with explicit signs.
144+
*/
145+
export function formatChurn(additions: number, deletions: number): string {
146+
return `+${additions} / -${deletions}`;
147+
}
148+
149+
/**
150+
* Normalize GitHub file status into summarized buckets.
151+
*/
152+
function normalizeStatus(status?: string): "added" | "modified" | "removed" | "renamed" | "other" {
153+
switch ((status || "").toLowerCase()) {
154+
case "added":
155+
return "added";
156+
case "removed":
157+
return "removed";
158+
case "renamed":
159+
return "renamed";
160+
case "modified":
161+
case "changed":
162+
case "copied":
163+
return "modified";
164+
default:
165+
return "modified";
166+
}
167+
}
168+
113169
interface ScopeRule {
114170
scope: ScopeKey;
115171
patterns: RegExp[];
@@ -145,7 +201,8 @@ const scopeRules: ScopeRule[] = [
145201
/^mrdocs-config\.cmake\.in$/i,
146202
],
147203
},
148-
{ scope: "tooling", patterns: [/^util\//i, /^tools\//i] },
204+
{ scope: "tooling", patterns: [/^tools\//i, /^util\/(?!danger\/)/i] },
205+
{ scope: "ci", patterns: [/^util\/danger\//i, /^\.github\//, /^\.roadmap\//] },
149206
{ scope: "third-party", patterns: [/^third-party\//i] },
150207
];
151208

@@ -181,20 +238,76 @@ function getScope(path: string): ScopeKey {
181238
*/
182239
export function summarizeScopes(files: FileChange[]): ScopeReport {
183240
const totals: Record<ScopeKey, ScopeTotals> = {
184-
"golden-tests": { scope: "golden-tests", files: 0, additions: 0, deletions: 0 },
185-
tests: { scope: "tests", files: 0, additions: 0, deletions: 0 },
186-
source: { scope: "source", files: 0, additions: 0, deletions: 0 },
187-
docs: { scope: "docs", files: 0, additions: 0, deletions: 0 },
188-
ci: { scope: "ci", files: 0, additions: 0, deletions: 0 },
189-
build: { scope: "build", files: 0, additions: 0, deletions: 0 },
190-
tooling: { scope: "tooling", files: 0, additions: 0, deletions: 0 },
191-
"third-party": { scope: "third-party", files: 0, additions: 0, deletions: 0 },
192-
other: { scope: "other", files: 0, additions: 0, deletions: 0 },
241+
"golden-tests": {
242+
scope: "golden-tests",
243+
files: 0,
244+
additions: 0,
245+
deletions: 0,
246+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
247+
},
248+
tests: {
249+
scope: "tests",
250+
files: 0,
251+
additions: 0,
252+
deletions: 0,
253+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
254+
},
255+
source: {
256+
scope: "source",
257+
files: 0,
258+
additions: 0,
259+
deletions: 0,
260+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
261+
},
262+
docs: {
263+
scope: "docs",
264+
files: 0,
265+
additions: 0,
266+
deletions: 0,
267+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
268+
},
269+
ci: {
270+
scope: "ci",
271+
files: 0,
272+
additions: 0,
273+
deletions: 0,
274+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
275+
},
276+
build: {
277+
scope: "build",
278+
files: 0,
279+
additions: 0,
280+
deletions: 0,
281+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
282+
},
283+
tooling: {
284+
scope: "tooling",
285+
files: 0,
286+
additions: 0,
287+
deletions: 0,
288+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
289+
},
290+
"third-party": {
291+
scope: "third-party",
292+
files: 0,
293+
additions: 0,
294+
deletions: 0,
295+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
296+
},
297+
other: {
298+
scope: "other",
299+
files: 0,
300+
additions: 0,
301+
deletions: 0,
302+
status: { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 },
303+
},
193304
};
194305

195306
let fileCount = 0;
196307
let additions = 0;
197308
let deletions = 0;
309+
const statusTotals = { added: 0, modified: 0, removed: 0, renamed: 0, other: 0 };
310+
const fileSummaries: FileSummary[] = [];
198311

199312
for (const file of files) {
200313
const scope = getScope(file.filename);
@@ -204,49 +317,49 @@ export function summarizeScopes(files: FileChange[]): ScopeReport {
204317
fileCount += 1;
205318
additions += file.additions || 0;
206319
deletions += file.deletions || 0;
207-
}
208320

209-
const scopesInOrder: ScopeKey[] = [
210-
"source",
211-
"tests",
212-
"golden-tests",
213-
"docs",
214-
"ci",
215-
"build",
216-
"tooling",
217-
"third-party",
218-
"other",
219-
];
321+
const normStatus = normalizeStatus(file.status);
322+
statusTotals[normStatus] += 1;
323+
totals[scope].status[normStatus] += 1;
324+
325+
fileSummaries.push({
326+
filename: file.filename,
327+
scope,
328+
additions: file.additions || 0,
329+
deletions: file.deletions || 0,
330+
churn: (file.additions || 0) + (file.deletions || 0),
331+
status: file.status,
332+
});
333+
}
220334

221-
const nonEmptyScopes = scopesInOrder.filter((scope) => totals[scope].files > 0);
222-
const header = "| Scope | Files | + / - |\n| --- | ---: | ---: |\n";
335+
const nonEmptyScopes = scopeDisplayOrder.filter((scope) => totals[scope].files > 0);
336+
const header = ["| Scope | Files | + / - |", "| --- | ---: | ---: |"].join("\n");
223337
const rows =
224338
nonEmptyScopes
225339
.map((scope) => {
226340
const scoped = totals[scope];
227-
return `| ${scope} | ${scoped.files} | +${scoped.additions} / -${scoped.deletions} |`;
341+
return `| ${scope} | **${scoped.files}** | **${formatChurn(scoped.additions, scoped.deletions)}** |`;
228342
})
229-
.join("\n") || "| (no changes) | 0 | +0 / -0 |";
230-
231-
const highlights = [];
232-
if (totals["golden-tests"].files > 0) {
233-
highlights.push("Golden test fixtures changed");
234-
}
235-
if (totals.tests.files === 0 && totals.source.files > 0) {
236-
highlights.push("Source updated without test coverage changes");
343+
.join("\n") || "| (no changes) | **0** | **+0 / -0** |";
344+
345+
const highlights: string[] = [];
346+
const goldenStatus = totals["golden-tests"].status;
347+
if (goldenStatus.modified > 0 || goldenStatus.renamed > 0) {
348+
highlights.push("Existing golden tests changed (behavior likely shifted)");
349+
} else if (goldenStatus.added > 0) {
350+
highlights.push("New golden tests added");
351+
} else if (goldenStatus.removed > 0) {
352+
highlights.push("Golden tests removed");
237353
}
238354

239-
const markdown = [
240-
"### Change summary by scope",
241-
`- Files changed: ${fileCount}`,
242-
`- Total churn: +${additions} / -${deletions}`,
243-
"",
244-
header + rows,
245-
].join("\n");
355+
const markdown = [header, rows].join("\n");
246356

247357
return {
248358
totals,
249-
overall: { files: fileCount, additions, deletions },
359+
overall: { files: fileCount, additions, deletions, status: statusTotals },
360+
topFiles: fileSummaries
361+
.sort((a, b) => b.churn - a.churn || a.filename.localeCompare(b.filename))
362+
.slice(0, 3),
250363
markdown,
251364
highlights,
252365
};
@@ -340,17 +453,20 @@ export function commitSizeWarnings(commits: CommitInfo[]): string[] {
340453
let churn = 0;
341454
for (const file of commit.files) {
342455
const scope = getScope(file.filename);
343-
if (scope === "tests" || scope === "golden-tests") {
456+
if (scope !== "source") {
344457
continue;
345458
}
346459
churn += (file.additions || 0) + (file.deletions || 0);
347460
}
348461

349-
if (churn > nonTestCommitLimit) {
462+
const summary = commit.message.split("\n")[0].trim();
463+
const parsedType = parseCommitSummary(summary || "")?.type;
464+
465+
if (churn > nonTestCommitLimit && parsedType !== "refactor") {
350466
const shortSha = commit.sha.substring(0, 7);
351467
// === Commit size warnings (non-test churn) ===
352468
messages.push(
353-
`Commit \`${shortSha}\` changes ${churn} non-test lines. Consider splitting it into smaller, reviewable chunks.`,
469+
`Commit \`${shortSha}\` (${summary}) changes ${churn} source lines. Consider splitting it into smaller, reviewable chunks.`,
354470
);
355471
}
356472
}
@@ -387,18 +503,17 @@ export function basicChecks(input: DangerInputs, scopes: ScopeReport, parsedComm
387503
if (cleanedBody.length < 40) {
388504
// === PR description completeness warnings ===
389505
warnings.push("PR description looks empty. Please add a short rationale and testing notes.");
390-
} else if (!/test(ed|ing)?/i.test(cleanedBody)) {
391-
// === Missing testing notes warnings ===
506+
} else if (
507+
scopes.totals.source.files > 0 &&
508+
scopes.totals["golden-tests"].files === 0 &&
509+
!/test(ed|ing)?/i.test(cleanedBody)
510+
) {
511+
// === Missing testing notes warnings (only when source changed and golden tests did not) ===
392512
warnings.push("Add a brief note about how this change was tested (or why tests are not needed).");
393513
}
394514

395515
const skipTests = hasSkipTests(input.prBody || "", input.labels);
396-
if (
397-
!skipTests &&
398-
scopes.totals.source.files > 0 &&
399-
scopes.totals.tests.files === 0 &&
400-
scopes.totals["golden-tests"].files === 0
401-
) {
516+
if (!skipTests && scopes.totals.source.files > 0 && scopes.totals.tests.files === 0 && scopes.totals["golden-tests"].files === 0) {
402517
// === Source changes without tests/fixtures warnings ===
403518
warnings.push(
404519
"Source changed but no tests or fixtures were updated. Add coverage or label with `no-tests-needed` / `[skip danger tests]` when appropriate.",

‎util/danger/run-local.ts‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import { readFileSync } from "fs";
1111
import path from "path";
1212
import { evaluateDanger, type DangerInputs } from "./logic";
13+
import { renderDangerReport } from "./format";
1314

1415
/**
1516
* Load a JSON fixture from disk and parse into DangerInputs.
@@ -29,19 +30,7 @@ function loadFixture(fixturePath: string): DangerInputs {
2930
* @param result evaluated Danger outputs to render.
3031
*/
3132
function printResult(result: ReturnType<typeof evaluateDanger>): void {
32-
console.log(result.summary.markdown);
33-
if (result.summary.highlights.length > 0) {
34-
console.log("\nHighlights:");
35-
for (const note of result.summary.highlights) {
36-
console.log(`- ${note}`);
37-
}
38-
}
39-
if (result.warnings.length > 0) {
40-
console.log("\nWarnings:");
41-
for (const message of result.warnings) {
42-
console.log(`- ${message}`);
43-
}
44-
}
33+
console.log(renderDangerReport(result));
4534
}
4635

4736
/**

0 commit comments

Comments
 (0)