Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check for uncommitted files beyond the diff directory #278

Merged
merged 1 commit into from Jul 25, 2023
+29 −14
Merged

Check for uncommitted files beyond the diff directory #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 15 additions & 3 deletions .github/workflows/check-dist.yml → .github/workflows/check-uncommitted.yml
View file
Open in desktop
@@ -1,4 +1,4 @@
name: Check dist
name: Check for uncommitted files

on:
pull_request:
Expand All @@ -8,7 +8,8 @@ on:
- 'releases/*'

jobs:
verify-build: # make sure the checked in dist/ folder matches the output of a rebuild
# This ensures a rebuild matches the checked-in dist/ folder
verify-build:
runs-on: ubuntu-latest

steps:
Expand All @@ -28,4 +29,15 @@ jobs:
run: npm run build

- name: Compare the expected and actual dist/ directories
run: bin/check-diff
run: bin/check-build-output-in-dist-directory
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I recall, the intent of using this script was to have the Check Run show the diff on failure so you didn't need to run it on your machine to see what the issue was.

This was useful as we had environment-specific issues early on where the Typescript compile wasn't properly encapsulated and produced a diff on Actions, but none locally.

It also checks for any delta in the git repo, not just the dist/ directory. Any change should fail CI until it's either committed or added to .gitignore.

That's fair - I don't have a strong opinion on this, I think it was originally just the dist/ directory to keep the CI build focused on verifying the distributed code as any other changes were less impactful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is useful, so I kept it, and then augmented with another job that looks for any other uncommitted files.


check-for-uncommitted-files:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha }}

- name: Compare the expected vs actual files
run: test -z "$(git status --porcelain)"
14 changes: 14 additions & 0 deletions bin/check-build-output-in-dist-directory
View file
Open in desktop
@@ -0,0 +1,14 @@
#!/bin/bash

# Make sure we notice any untracked files generated by the build in the dist/ directory
git add --intent-to-add .
git diff --quiet dist/
retVal=$?
if [ $retVal -ne 0 ]; then
echo "Detected uncommitted changes after build:"
# The contents of the diff/ folder are marked as generated:
# https://github.com/dependabot/fetch-metadata/blob/6c2bf2fe33cc133b474165107a8b29ccc265dc96/.gitattributes#L1
# so this ensures we spit out the actual change in the obfuscated JS.
git --no-pager diff dist/
exit 1
fi
11 changes: 0 additions & 11 deletions bin/check-diff
View file
Open in desktop

This file was deleted.