Skip to content

check if the deploy directory is empty or not #5

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

Merged
merged 1 commit into from
Jan 19, 2015
Merged

check if the deploy directory is empty or not #5

merged 1 commit into from
Jan 19, 2015

Conversation

txchen
Copy link
Contributor

@txchen txchen commented Dec 30, 2014

An empty deploy directory would generate unexpected results:

  • It may wipe out gh-pages branch, usually we don't expect that.
  • In a newly cloned repo, if the folder does not exist, script would
    get error like:
    fatal: This operation must be run in a work tree
    The error is from line "git --work-tree "$deploy_directory" reset
    --mixed --quiet"
  • If remote branch does not exist, 'deploy.sh -s' would also get wrong
    result.

To improve it, check if the directory is not empty in the beginning.

An empty deploy directory would generate unexpected results:
* It may wipe out gh-pages branch, usually we don't expect that.
* In a newly cloned repo, if the folder does not exist, script would
  get error like:
  fatal: This operation must be run in a work tree
  The error is from line "git --work-tree "$deploy_directory" reset
  --mixed --quiet"
* If remote branch does not exist, 'deploy.sh -s' would also get wrong
  result.

To improve it, check if the directory is not empty in the beginning.
@txchen
Copy link
Contributor Author

txchen commented Dec 30, 2014

Thanks for making this great script. It fits very well for non-jekyll github pages scenarios!

When I try to play with the script, I found that if the target dir does not exist or if it is empty, we might get some weird results. I think it is better to check if the folder has some files in the beginning, details in the commit message, thanks a lot!

@X1011 X1011 mentioned this pull request Jan 19, 2015
9 tasks
@X1011
Copy link
Owner

X1011 commented Jan 19, 2015

glad my script is helpful!

  1. i'm not sure i follow how it may wipe out the gh-pages branch. could you give me an example?
  2. yes, that's not a friendly error. thanks for the PR. i'll push it to master after a couple tweaks.
  3. yes, that probably doesn't work. we should have tests for these edge cases, so i've created issue add tests #6: add tests, one of which should test "non-existent remote deploy_branch". i don't know if or when i'll get around to this, as i'm not even currently using this script.

@X1011
Copy link
Owner

X1011 commented Jan 19, 2015

oh, i think i know what you mean for the 1st one. it will wipe out the contents of deploy_branch, not the deploy_branch itself. i'll check that as a separate condition and let the user override with a flag.

@X1011 X1011 merged commit ae428e2 into X1011:master Jan 19, 2015
X1011 added a commit that referenced this pull request Jan 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants