heroku-review-app-actions
heroku-review-app-actions copied to clipboard
Fix missing files, using files from git ls
This PR was inspired by #5 (thank you @pierrickouw for helping me realise why my build was failing!), but the suggested fix in #5 gives me this error message:
tar: .: file changed as we read it
Looking at the logs I can see that tar
was trying to include the .git
directory, and other unnecessary files.
Using the list of files from git instead seems like it would correspond better to what the GitHub integration would normally be sending to Herokuโthis filters out anything that would be git ignored.
That's way better, closing my PR! Thank you!
Thanks for sending PR ๐๐ฝ ๐๐ฝ @ch2ch3 and @pierrickouw
I still have hesitation using git ls-files
as it is at the moment. This will include files in .github
, include .gitignore
, etc. ATM I think it would be better to have an input parameter (probably using glob) for a list of files/directories to include or exclude.
Then it could be implemented somewhat like tar -czvf --exclude='./folder' --exclude='./folder2' --exclude='file.ext' source.tar.gz $(git ls-files)
.
What do you think?
@pmbanugo I hear your concern!
Personally, I'm not too worried about those files as they are usually small and thus negligible to include in the compressed source code. On the flip side, there could be a risk of someone not realising that they have to "reimplement" git ignore, and accidentally sending secrets or large binaries to Heroku which aren't needed. Maybe git ls-files
could be the default, but with the option to override for those who want more control over what to include/exclude? I will leave it to you to make the call ๐
P.S. I updated the base branch as I belatedly noticed the note in your README about sending contributions to the dev
branch ๐
Just wanted to mention that I fix the same issue in a fork using git archive -o source.tar.gz HEAD
(cf. https://www.atlassian.com/git/tutorials/export-git-archive)
Thanks for all the contributions. I decided to use the option to pass in a glob pattern as input for now. I'm leaving this PR open while I observe how it gets used and reconsider this proposal again in the future.