Check for uncommitted files beyond the diff directory
#278
Conversation
| @@ -31,5 +32,7 @@ jobs: | |||
| - name: Rebuild the dist/ directory | |||
| run: npm run build | |||
|
|
|||
| - name: Compare the expected and actual dist/ directories | |||
| run: bin/check-diff | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This is useful, so I kept it, and then augmented with another job that looks for any other uncommitted files.
| retVal=$? | ||
| if [ $retVal -ne 0 ]; then | ||
| echo "Detected uncommitted changes after build:" | ||
| git --no-pager diff dist/ |
There was a problem hiding this comment.
The contents of the diff/ folder are marked as generated so this was just to ensure we spat out the actual change in the obfuscated JS.
|
Hmm... I'm really not sure what to do here, as @brrygrdn raises good points, but I'm still not excited by having a separate script... that said, movign the I'll let this float a bit long in case anyone else has opinions, otherwise I'll probably either close this or leave existing code as-is, but add a follow-on step so that if there are no changes in I guess what also could be helpful here is a few more code comments in the source so that someone down the road doesn't open a PR on this either. |
jeffwidman
force-pushed
the
replace-check-diff-with-a-one-liner
branch
from
cd41e39
to
02f6890
Compare
There was a problem hiding this comment.
Code didn't change in this script, only renamed it and added code comments for improved clarity.
jeffwidman
changed the title
bin/check-diff with a one-linerdiff directory
|
I updated based on @brrygrdn 's helpful context/feedback. This is ready for another review. |
This 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`. Additionally, I clarified the script name/code slightly to explain why it's needed/handled separately from checking for uncommitted files.
jeffwidman
force-pushed
the
replace-check-diff-with-a-one-liner
branch
from
02f6890
to
de83502
Compare
This removes the indirection of the bash script so that it's immediately clear from the github action definition file what is being checked. Currently the logic is split between the action which does the rebuild and the bash script which checks the diff.
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.