Easy does it
October 5, 2023•752 words
(Let's see if this blogging software works.)
At Medium (and, quite honestly, at every other company I worked at) there were a lot of lint errors. We didn't want to ignore them completely, but we also didn't want to spend weeks trying to fix them all. So we wrote a little shell script to count the number of errors and complain if your PR increased that number.
I don't have that script anymore, but it was probably something like this:
#!/bin/bash EXP_ERRORS=123 num_errors=$(eslint 2> /dev/null | grep ^src/ | wc -l | awk '$1=$1') if [ "$num_errors" -gt "$EXP_ERRORS" ]; then echo "You added lint errors, please fix them before committing." exit 1 else if [ "$num_errors" -lt "$EXP_ERRORS" ]; then echo "You fixed lint errors. Great job! Please update EXP_ERRORS in this/script.sh" exit 1 fi Was it annoying to modify variables to get CI to pass? Yes, but it also worked.
At Kodex, we had a similar issue with TypeScript and lint errors, so I went to do the same thing. This time, however, a coworker suggested I use GitHub Actions Artifacts instead of making people modify variables. I think it worked out pretty well:
name: CI on: pull_request: types: [opened, synchronize, reopened, ready_for_review] defaults: run: working-directory: ./client jobs: test-base: runs-on: gh-hosted-action-runner steps: - uses: actions/checkout@v4 with: ref: main fetch-depth: 2 - uses: actions/setup-node@v3 with: node-version: "18.17.0" - name: Install Yarn run: npm i -g yarn - name: Install modules run: yarn # For whatever reason, if I put these commands with a redirect (>) # into a script, they don't create the files. - name: Create baseline files run: | npx tsc --noEmit 2> /dev/null | grep ^src/ | wc -l | awk '$1=$1' > ./EXISTING_TSC_ERRORS npx eslint --quiet 'src/**/*.{ts,tsx,js,jsx}' src/ 2> /dev/null | grep -E '^[[:space:]]*[0-9]+:[0-9]+[[:space:]]+error' | wc -l | awk '$1=$1' > ./EXISTING_LINT_ERRORS - uses: actions/upload-artifact@v3 with: name: EXISTING_TSC_ERRORS path: ./client/EXISTING_TSC_ERRORS - uses: actions/upload-artifact@v3 with: name: EXISTING_LINT_ERRORS path: ./client/EXISTING_LINT_ERRORS test-client: runs-on: gh-hosted-action-runner needs: test-base steps: - uses: actions/checkout@v4 with: fetch-depth: 2 - uses: actions/setup-node@v3 with: node-version: "18.17.0" - name: Install Yarn run: npm i -g yarn - name: Install modules run: yarn - uses: actions/download-artifact@v3 with: name: EXISTING_TSC_ERRORS path: ./client - uses: actions/download-artifact@v3 with: name: EXISTING_LINT_ERRORS path: ./client - name: Run typecheck run: bash ./scripts/typecheck.sh - name: Run tests run: yarn test The typecheck.sh is pretty simple:
#!/bin/bash # Runs the TypeScript compiler (in type-checking mode) and ESLint, # counts the numbers of errors and exits with a non-zero code if # the number of errors it higher than acceptable, as defined by # EXISTING_TSC_ERRORS and EXISTING_LINT_ERRORS artifacts. # # The idea is to not add more errors and ideally gradually ratchet # down existing errors to zero. exit_code=0 echo -n "tsc... " num_tsc_errors=$(npx tsc --noEmit 2> /dev/null | grep ^src/ | wc -l | awk '$1=$1') exp_tsc_errors=$(cat EXISTING_TSC_ERRORS) if [ "$num_tsc_errors" -gt "$exp_tsc_errors" ]; then echo "errors increased from $exp_tsc_errors to $num_tsc_errors." exit_code=1 else echo "ok" fi echo -n "eslint... " num_eslint_errors=$(npx eslint --quiet 'src/**/*.{ts,tsx,js,jsx}' src/ 2> /dev/null | grep -E '^[[:space:]]*[0-9]+:[0-9]+[[:space:]]+error' | wc -l | awk '$1=$1') exp_eslint_errors=$(cat EXISTING_LINT_ERRORS) if [ "$num_eslint_errors" -gt "$exp_eslint_errors" ]; then echo "errors increased from $exp_eslint_errors to $num_eslint_errors." exit_code=1 else echo "ok" fi exit $exit_code What it does
- On each PR, it checks out the main branch, runs the linter, counts the errors, and saves the number to a file
- Using
actions/upload-artifact, it uploads that file as an artifact. - Then it checks out the PR branch and downloads that artifact using
actions/download-artifact. - Finally, it runs the linter again, counts the errors, and compares the numbers.
- If the number is higher, it errors. If the number is equal or lower, there's nothing to do.
In the future, I might try modifying the script to use the merge base, instead of the main branch, and saving the actual linter/compiler output instead of counting errors so I could run a diff on it and show more helpful errors.
Gotchas
actions/download-artifactandactions/upload-artifactignore the working directory so your path has to include it.- For whatever reason I couldn't figure out why redirecting output to a file within a script prevented
upload-artifactfrom picking up the file. - On each PR you have to run your checks twice. For us, installing dependencies and running a linter is pretty fast so it's worth it. If it's a bottleneck for you consider using artifacts or actions cache to save dependencies between jobs.