Skip to content

Commit 7ee9e0b

Browse files
committed
core: improve the diff github comment
This commit improves the github diff comment on PRs by: - Stylize the comment title as a h3 (instead of normal text) - Add the slug of the doc (including the hub slug and the branch slug) in the comment title - Move the preview doc link before the change details - Put change details in a collapsible section (opened by default) - Smaller “powered by bump.sh” footer
1 parent 7627513 commit 7ee9e0b

File tree

5 files changed

+88
-39
lines changed

5 files changed

+88
-39
lines changed

src/diff.ts

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,40 @@ export async function run(diff: DiffResponse, repo: Repo): Promise<void> {
88
digestContent.push(diff.public_url);
99
}
1010
const digest = shaDigest(digestContent);
11-
const body = buildCommentBody(repo.docDigest, diff, digest);
11+
const body = buildCommentBody(repo, diff, digest);
1212

1313
return repo.createOrUpdateComment(body, digest);
1414
}
1515

16-
function buildCommentBody(docDigest: string, diff: DiffResponse, digest: string) {
16+
function buildCommentBody(repo: Repo, diff: DiffResponse, digest: string) {
1717
const emptySpace = '';
18-
const poweredByBump = '> _Powered by [Bump.sh](https://bump.sh)_';
19-
const text = diff.markdown || 'No structural change, nothing to display.';
18+
const poweredByBump = '###### _Powered by [Bump.sh](https://bump.sh)_';
19+
let text = 'No structural change, nothing to display.';
20+
if (diff.markdown) {
21+
text = `<details open><summary>Structural change details</summary>
2022
21-
return [title(diff)]
22-
.concat([emptySpace, text])
23-
.concat([viewDiffLink(diff), poweredByBump, bumpDiffComment(docDigest, digest)])
23+
${diff.markdown}
24+
25+
</details>`;
26+
}
27+
28+
return [title(diff, repo.doc, repo.hub, repo.branch)]
29+
.concat([viewDiffLink(diff)])
30+
.concat([text, emptySpace])
31+
.concat([poweredByBump, bumpDiffComment(repo.docDigest, digest)])
2432
.join('\n');
2533
}
2634

27-
function title(diff: DiffResponse): string {
28-
const structureTitle = '🤖 API structural change detected:';
29-
const contentTitle = 'ℹ️ API content change detected:';
30-
const breakingTitle = '🚨 Breaking API change detected:';
35+
function title(diff: DiffResponse, doc: string, hub?: string, branch?: string): string {
36+
let docName = [hub, doc].filter((e) => e).join('/');
37+
// Capitalize doc name
38+
docName = docName.charAt(0).toUpperCase() + docName.slice(1);
39+
if (branch) {
40+
docName = `${docName} (branch: ${branch})`;
41+
}
42+
const structureTitle = `### 🤖 ${docName} API structural change detected`;
43+
const contentTitle = `### ℹ️ ${docName} API content change detected`;
44+
const breakingTitle = `### 🚨 Breaking ${docName} API change detected`;
3145

3246
if (diff.breaking) {
3347
return breakingTitle;

src/github.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ export class Repo {
2323
readonly baseSha?: string;
2424
readonly headSha?: string;
2525
private _docDigest: string;
26+
readonly doc: string;
27+
readonly hub: string | undefined;
28+
readonly branch: string | undefined;
2629

2730
constructor(doc: string, hub?: string, branch?: string) {
2831
// Fetch GitHub Action context
@@ -38,6 +41,9 @@ export class Repo {
3841
this.headSha = pull_request.head.sha;
3942
}
4043
this.octokit = this.getOctokit();
44+
this.doc = doc;
45+
this.hub = hub;
46+
this.branch = branch;
4147
this._docDigest = shaDigest([doc, hub, branch]);
4248
}
4349

tests/diff.test.ts

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,22 +43,26 @@ describe('diff.ts', () => {
4343
};
4444
const digest = '4b81e612cafa6580f8ad3bfe9e970b2d961f58c2';
4545

46-
const repo = new Repo('hello');
47-
const docDigest = shaDigest(['hello']);
46+
const repo = new Repo('hello', '', 'v2');
47+
const docDigest = shaDigest(['hello', '', 'v2']);
4848

4949
await diff.run(result, repo);
5050

5151
expect(repo.createOrUpdateComment).toHaveBeenCalledWith(
52-
`🤖 API structural change detected:
52+
`### 🤖 Hello (branch: v2) API structural change detected
53+
54+
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
55+
56+
<details open><summary>Structural change details</summary>
5357
5458
* one
5559
* two
5660
* three
5761
5862
59-
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
63+
</details>
6064
61-
> _Powered by [Bump.sh](https://bump.sh)_
65+
###### _Powered by [Bump.sh](https://bump.sh)_
6266
<!-- Bump.sh digest=${digest} doc=${docDigest} -->`,
6367
digest,
6468
);
@@ -72,18 +76,18 @@ describe('diff.ts', () => {
7276
};
7377
const digest = '3999a0fe6ad27841bd6342128f7028ab2cea1c57';
7478

75-
const repo = new Repo('hello');
76-
const docDigest = shaDigest(['hello']);
79+
const repo = new Repo('hello', 'my-hub', 'v1');
80+
const docDigest = shaDigest(['hello', 'my-hub', 'v1']);
7781
await diff.run(result, repo);
7882

7983
expect(repo.createOrUpdateComment).toHaveBeenCalledWith(
80-
`ℹ️ API content change detected:
81-
82-
No structural change, nothing to display.
84+
`### ℹ️ My-hub/hello (branch: v1) API content change detected
8385
8486
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
8587
86-
> _Powered by [Bump.sh](https://bump.sh)_
88+
No structural change, nothing to display.
89+
90+
###### _Powered by [Bump.sh](https://bump.sh)_
8791
<!-- Bump.sh digest=${digest} doc=${docDigest} -->`,
8892
digest,
8993
);
@@ -105,16 +109,20 @@ No structural change, nothing to display.
105109
await diff.run(result, repo);
106110

107111
expect(repo.createOrUpdateComment).toHaveBeenCalledWith(
108-
`🚨 Breaking API change detected:
112+
`### 🚨 Breaking Hello API change detected
113+
114+
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
115+
116+
<details open><summary>Structural change details</summary>
109117
110118
* one
111119
* two
112120
* three
113121
114122
115-
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
123+
</details>
116124
117-
> _Powered by [Bump.sh](https://bump.sh)_
125+
###### _Powered by [Bump.sh](https://bump.sh)_
118126
<!-- Bump.sh digest=${digest} doc=${docDigest} -->`,
119127
digest,
120128
);
@@ -136,14 +144,18 @@ No structural change, nothing to display.
136144
await diff.run(result, repo);
137145

138146
expect(repo.createOrUpdateComment).toHaveBeenCalledWith(
139-
`🤖 API structural change detected:
147+
`### 🤖 Hello API structural change detected
148+
149+
<details open><summary>Structural change details</summary>
140150
141151
* one
142152
* two
143153
* three
144154
145155
146-
> _Powered by [Bump.sh](https://bump.sh)_
156+
</details>
157+
158+
###### _Powered by [Bump.sh](https://bump.sh)_
147159
<!-- Bump.sh digest=${digest} doc=${docDigest} -->`,
148160
digest,
149161
);

tests/fixtures/repo.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export const mockCreateOrUpdateComment = jest.fn<repo.Repo['createOrUpdateCommen
66
export const mockDeleteExistingComment = jest.fn<repo.Repo['deleteExistingComment']>();
77
export const Repo = jest.fn().mockImplementation(() => {
88
return {
9+
doc: 'my-doc',
910
getBaseFile: mockGetBaseFile,
1011
createOrUpdateComment: mockCreateOrUpdateComment,
1112
deleteExistingComment: mockDeleteExistingComment,

tests/main.test.ts

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,17 @@ describe('main.ts', () => {
267267
expect(repo.Repo).toHaveBeenCalledWith('my-doc', '', '');
268268
expect(repo.mockGetBaseFile).toHaveBeenCalledWith(file);
269269
expect(repo.mockCreateOrUpdateComment).toHaveBeenCalledWith(
270-
`🚨 Breaking API change detected:
270+
`### 🚨 Breaking My-doc API change detected
271+
272+
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
273+
274+
<details open><summary>Structural change details</summary>
271275
272276
one
273277
274-
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
278+
</details>
275279
276-
> _Powered by [Bump.sh](https://bump.sh)_
280+
###### _Powered by [Bump.sh](https://bump.sh)_
277281
<!-- Bump.sh digest=${commentDigest} doc=undefined -->`,
278282
commentDigest,
279283
);
@@ -303,13 +307,17 @@ one
303307
expect(repo.Repo).toHaveBeenCalledWith('my-doc', '', '');
304308
expect(repo.mockGetBaseFile).toHaveBeenCalledWith(file);
305309
expect(repo.mockCreateOrUpdateComment).toHaveBeenCalledWith(
306-
`🚨 Breaking API change detected:
310+
`### 🚨 Breaking My-doc API change detected
311+
312+
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
313+
314+
<details open><summary>Structural change details</summary>
307315
308316
one
309317
310-
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
318+
</details>
311319
312-
> _Powered by [Bump.sh](https://bump.sh)_
320+
###### _Powered by [Bump.sh](https://bump.sh)_
313321
<!-- Bump.sh digest=${commentDigest} doc=undefined -->`,
314322
commentDigest,
315323
);
@@ -337,13 +345,17 @@ one
337345
expect(repo.Repo).toHaveBeenCalledWith('', '', 'latest');
338346
expect(repo.mockGetBaseFile).toHaveBeenCalledWith(file);
339347
expect(repo.mockCreateOrUpdateComment).toHaveBeenCalledWith(
340-
`🚨 Breaking API change detected:
348+
`### 🚨 Breaking My-doc API change detected
349+
350+
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
351+
352+
<details open><summary>Structural change details</summary>
341353
342354
one
343355
344-
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
356+
</details>
345357
346-
> _Powered by [Bump.sh](https://bump.sh)_
358+
###### _Powered by [Bump.sh](https://bump.sh)_
347359
<!-- Bump.sh digest=${commentDigest} doc=undefined -->`,
348360
commentDigest,
349361
);
@@ -372,13 +384,17 @@ one
372384

373385
expect(repo.Repo).toHaveBeenCalledWith('', '', '');
374386
expect(repo.mockCreateOrUpdateComment).toHaveBeenCalledWith(
375-
`🚨 Breaking API change detected:
387+
`### 🚨 Breaking My-doc API change detected
388+
389+
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
390+
391+
<details open><summary>Structural change details</summary>
376392
377393
one
378394
379-
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
395+
</details>
380396
381-
> _Powered by [Bump.sh](https://bump.sh)_
397+
###### _Powered by [Bump.sh](https://bump.sh)_
382398
<!-- Bump.sh digest=${commentDigest} doc=undefined -->`,
383399
commentDigest,
384400
);

0 commit comments

Comments
 (0)