-
Notifications
You must be signed in to change notification settings - Fork 191
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.
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
Fix remove not working properly #336
Conversation
I encountered this bug also #337, i hope this will be merged |
Thanks for the proposed fix, @sunghwan2789. Can you please remove the added |
Oh I did not notice that. Removed now! |
@@ -31,7 +31,7 @@ exports.defaults = { | |||
branch: 'gh-pages', | |||
remote: 'origin', | |||
src: '**/*', | |||
only: '.', | |||
remove: '.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This default scares me. I haven't found time to do a more thorough review, but I don't get why we would want to remove everything by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scratch that, I see that is the currently documented behavior. Clearly I need to get my head back into this before understanding if this change does the right thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think removing all files default seems ok to me, as options.add
is present for controlling this behaviour.
For a reference, please take a look https://github.com/marketplace/actions/github-pages-action#%EF%B8%8F-keeping-existing-files
Thanks for your work on this, @sunghwan2789. |
This PR fixes some issues described below and resolves #334, fixes #337 .
Remove globbed files
gh-pages/lib/index.js
Line 156 in 3579ab2
Joining files in a string causes incorrect shell argument passing. A globbed file can be removed, but globbed files cannot be removed since the actual command will be
This PR makes
git.rm
andgit.add
acceptstring[]
and handle them properly.Remove files in remote dist directory, not in local one
gh-pages/lib/index.js
Lines 97 to 99 in 3579ab2
This shows that gh-pages globs files to be removed in local dist directory, not in remote branch. It causes git a fatal error(like #337) since it tries to remove files that are not in remote dist directory.
globby.sync
should be executed in remote dist directory.Skip removing files if there are no globbed files
When
globby.sync
globs files, it returns empty array if there are no files to be removed. A guard of it should be added beforegh-pages/lib/index.js
Line 156 in 3579ab2