Skip to content

Commit d25b83c

Browse files
alicewriteswrongsalicewriteswrongs
authored
chore(ci): update tech debt burndown report to include unused exports (stenciljs#3372)
this adds a report to our CI check which prints out a little bit of info about the number of unused exports in the code, so that we'll get around to removing them!
1 parent c49ee1a commit d25b83c

File tree

4 files changed

+238
-96
lines changed

4 files changed

+238
-96
lines changed

‎.github/workflows/strict-null-checks.yml

-90
This file was deleted.

‎.github/workflows/tech-debt-burndown.yml

+142
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
name: Tech Debt Burndown
2+
# this workflow is for reporting on various metrics for the codebase that
3+
# we want to pay attention to. Generally these are checks of some sort that we'll
4+
# want to eventually 'graduate' to full CI checks (which cause builds to fail if
5+
# there are any errors) once we've eliminated all the problems, but until that
6+
# point we run them here, separate from the main build, and write a report on our
7+
# progress on them to each PR.
8+
9+
on:
10+
pull_request:
11+
branches:
12+
- '**'
13+
14+
jobs:
15+
strict_null_check: # TODO(STENCIL-446): Remove this workflow once `strictNullChecks` is enabled
16+
strategy:
17+
matrix:
18+
branch: [ 'main', 'pr' ]
19+
name: 'Get strictNullChecks errors on ${{ matrix.branch }}'
20+
runs-on: 'ubuntu-latest'
21+
steps:
22+
- name: Checkout main
23+
uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 # v2.4.0
24+
with:
25+
ref: main
26+
if: ${{ matrix.branch == 'main' }}
27+
28+
- name: Checkout PR branch
29+
uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 # v2.4.0
30+
if: ${{ matrix.branch == 'pr' }}
31+
32+
- name: Get Core Dependencies
33+
uses: ./.github/workflows/actions/get-core-dependencies
34+
35+
- name: Install tsc-output-parser
36+
run: npm install @aivenio/[email protected]
37+
38+
- name: Run Typescript compiler and generate JSON-formatted error file
39+
run: npx tsc --strictNullChecks --noEmit --pretty false | npx tsc-output-parser > null_errors_${{ matrix.branch }}.json
40+
41+
- name: Upload null_errors_${{ matrix.branch }}.json
42+
uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 # v3.0.0
43+
with:
44+
name: null_errors_${{ matrix.branch }}
45+
path: 'null_errors_${{ matrix.branch }}.json'
46+
47+
# TODO(STENCIL-454): Remove or change this up once we've eliminated unused exports
48+
unused_exports_check:
49+
strategy:
50+
matrix:
51+
branch: [ 'main', 'pr' ]
52+
name: Find unused variables on ${{ matrix.branch }}
53+
runs-on: 'ubuntu-latest'
54+
steps:
55+
- name: Checkout main
56+
uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 # v2.4.0
57+
with:
58+
ref: main
59+
if: ${{ matrix.branch == 'main' }}
60+
61+
- name: Checkout PR branch
62+
uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 # v2.4.0
63+
if: ${{ matrix.branch == 'pr' }}
64+
65+
- name: Install ts-prune
66+
run: npm install [email protected]
67+
68+
- name: Run ts-prune and write output to disk
69+
run: npx ts-prune > unused-exports-${{ matrix.branch }}.txt
70+
71+
- name: Upload unused exports
72+
uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 # v3.0.0
73+
with:
74+
name: unused-exports-${{ matrix.branch }}
75+
path: 'unused-exports-${{ matrix.branch }}.txt'
76+
77+
format_report:
78+
needs: [ "strict_null_check", "unused_exports_check" ]
79+
name: Download error files and report
80+
runs-on: 'ubuntu-latest'
81+
steps:
82+
- name: Checkout current branch
83+
uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 # v2.4.0
84+
85+
- name: Get Core Dependencies
86+
uses: ./.github/workflows/actions/get-core-dependencies
87+
88+
# TODO(STENCIL-446): Remove this workflow once `strictNullChecks` is enabled
89+
- name: Download null errors file for main branch
90+
uses: actions/download-artifact@fb598a63ae348fa914e94cd0ff38f362e927b741 # v3.0.0
91+
with:
92+
name: null_errors_main
93+
94+
# TODO(STENCIL-446): Remove this workflow once `strictNullChecks` is enabled
95+
- name: Download null errors file for PR
96+
uses: actions/download-artifact@fb598a63ae348fa914e94cd0ff38f362e927b741 # v3.0.0
97+
with:
98+
name: null_errors_pr
99+
100+
# TODO(STENCIL-454): Remove or change this up once we've eliminated unused exports
101+
- name: Download unused exports for main
102+
uses: actions/download-artifact@fb598a63ae348fa914e94cd0ff38f362e927b741 # v3.0.0
103+
with:
104+
name: unused-exports-main
105+
106+
# TODO(STENCIL-454): Remove or change this up once we've eliminated unused exports
107+
- name: Download unused exports for PR
108+
uses: actions/download-artifact@fb598a63ae348fa914e94cd0ff38f362e927b741 # v3.0.0
109+
with:
110+
name: unused-exports-pr
111+
112+
- name: Compile scripts
113+
run: node scripts --prepare
114+
115+
- name: Set action output
116+
run: node scripts/build/tech-debt-burndown-report.js > $GITHUB_STEP_SUMMARY
117+
118+
# for syntax information, see https://github.com/peter-evans/create-or-update-comment#setting-the-comment-body-from-a-file
119+
- name: Set comment body
120+
id: set-comment-body
121+
run: |
122+
body=$(node scripts/build/tech-debt-burndown-report.js)
123+
body="${body//'%'/'%25'}"
124+
body="${body//$'\n'/'%0A'}"
125+
body="${body//$'\r'/'%0D'}"
126+
echo ::set-output name=body::$body
127+
128+
- name: Find Comment
129+
uses: peter-evans/find-comment@1769778a0c5bd330272d749d12c036d65e70d39d # v2.0.0
130+
id: fc
131+
with:
132+
issue-number: ${{ github.event.pull_request.number }}
133+
comment-author: 'github-actions[bot]'
134+
body-includes: '### `--strictNullChecks` error report'
135+
136+
- name: Create or update comment
137+
uses: peter-evans/create-or-update-comment@c9fcb64660bc90ec1cc535646af190c992007c32 # v2.0.0
138+
with:
139+
comment-id: ${{ steps.fc.outputs.comment-id }}
140+
issue-number: ${{ github.event.pull_request.number }}
141+
body: ${{ steps.set-comment-body.outputs.body }}
142+
edit-mode: replace

‎.gitignore

+2
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,5 @@ coverage/**
5050

5151
# TODO(STENCIL-446): Remove these once `strictNullChecks` is enabled
5252
null_errors*.json
53+
# TODO(STENCIL-454): Remove or change this up once we've eliminated unused exports
54+
unused-exports*.txt

‎scripts/strict-null-checks-ci.ts ‎scripts/tech-debt-burndown-report.ts

+94-6
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
/**
2-
* TODO(STENCIL-446): Remove this script once `strictNullChecks` is enabled
2+
* A script for formatting a Markdown report for CI on some quantities we're tracking as we work
3+
* on tech debt on the current branch vs on main. The report includes some info about strictNullChecks errors,
4+
* as well as tracking unused (dead) code.
35
*/
6+
import fs from 'fs';
47

8+
//// StrictNullChecks error checking ////
59
/**
6-
* A script for formatting a Markdown report for CI on the number of strictNullChecks errors we're
7-
* seeing on the current branch vs on main. The report also includes some info about the most
8-
* error-filled files, as well as the errors we see most often.
10+
* TODO(STENCIL-446): Remove this section once `strictNullChecks` is enabled
911
*/
1012

11-
import fs from 'fs';
12-
1313
/**
1414
* This interface is copied (a bit) from here:
1515
* https://github.com/aiven/tsc-output-parser/blob/master/src/parserTypes.ts
@@ -115,6 +115,59 @@ const fileErrorCounts = countArrayEntries(prData.map((error) => error.value.path
115115

116116
const errorCodeCounts = countArrayEntries(prData.map((error) => error.value.tsError.value.errorString));
117117

118+
//// unused exports check ////
119+
// TODO(STENCIL-454): Remove or change up this report once we've eliminated unused exports
120+
121+
/**
122+
* ts-prune includes this string on lines in its output corresponding to items
123+
* which are exported and not imported anywhere but which *are* used in their
124+
* home modules. We want to exclude these and get only the truly dead code.
125+
*/
126+
const USED_IN_MODULE = '(used in module)';
127+
128+
/**
129+
* Load in data from the unused exports reports
130+
*/
131+
const unusedExportsMain = String(fs.readFileSync('./unused-exports-main.txt'));
132+
const unusedExportsPR = String(fs.readFileSync('./unused-exports-pr.txt'));
133+
134+
/**
135+
* A little record of a location of putative dead code
136+
*/
137+
interface DeadCodeLoc {
138+
fileName: string;
139+
lineNumber: string;
140+
identifier: string;
141+
}
142+
143+
/**
144+
* Process and format the raw output from ts-prune into a more coherent format.
145+
* @param rawFileContents the unprocessed contents of the file
146+
* @returns an array of dead code location records.
147+
*/
148+
function processUnusedExports(rawFileContents: string): DeadCodeLoc[] {
149+
const deadCodeLines = rawFileContents
150+
.trim()
151+
.split('\n')
152+
.filter((line) => !line.includes(USED_IN_MODULE));
153+
154+
return deadCodeLines.map((line) => {
155+
// lines which are _not_ used in their home module are formatted like this:
156+
// path/to/module.ts:33 - identifierName
157+
const [fileAndLineNumber, identifier] = line.split(' - ');
158+
const [fileName, lineNumber] = fileAndLineNumber.split(':');
159+
160+
return {
161+
fileName,
162+
lineNumber,
163+
identifier,
164+
};
165+
});
166+
}
167+
168+
const deadCodeMain = processUnusedExports(unusedExportsMain);
169+
const deadCodePR = processUnusedExports(unusedExportsPR);
170+
118171
// Markdown formatting helpers
119172

120173
/**
@@ -244,4 +297,39 @@ lines.push(
244297
})
245298
);
246299

300+
lines.push('');
301+
302+
const deadCodeCountPR = deadCodePR.length;
303+
const deadCodeCountMain = deadCodeMain.length;
304+
305+
lines.push('### Unused exports report');
306+
lines.push('');
307+
308+
const deadCodeLine = [];
309+
deadCodeLine.push(`There are ${deadCodePR.length} unused exports on this PR. `);
310+
311+
// we can check the number of errors just to write a different message out here
312+
// depending on the delta between this branch and main
313+
if (deadCodeCountPR === deadCodeCountMain) {
314+
deadCodeLine.push("That's the same number of errors on main, so at least we're not creating new ones!");
315+
} else if (deadCodeCountPR < deadCodeCountMain) {
316+
deadCodeLine.push(`That's ${deadCodeCountMain - deadCodeCountPR} fewer than on \`main\`! 🎉🎉🎉`);
317+
} else {
318+
deadCodeLine.push(
319+
`Unfortunately, it looks like that's an increase of ${deadCodeCountPR - deadCodeCountMain} over \`main\` 😞.`
320+
);
321+
}
322+
lines.push(deadCodeLine.join(''));
323+
324+
lines.push('');
325+
lines.push(
326+
collapsible('Unused exports', (out) => {
327+
out.push(tableHeader('File', 'Line', 'Identifier'));
328+
329+
deadCodePR.forEach((deadCode) => {
330+
out.push(tableRow(deadCode.fileName, deadCode.lineNumber, deadCode.identifier));
331+
});
332+
})
333+
);
334+
247335
console.log(lines.join('\n'));

0 commit comments

Comments
 (0)