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.

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

Use set -e to fail fast #111

Merged
merged 1 commit into from
Jul 5, 2015
Merged

Use set -e to fail fast #111

merged 1 commit into from
Jul 5, 2015

Conversation

5long
Copy link
Contributor

@5long 5long commented Mar 21, 2013

Hi, I just got started to use mina today. And I noticed that even if some command when deploying exits with a non-zero status, the following commands would still execute. I really don't want that to happen since I simply don't want to know what would happen in that situation.

This patch makes use of bash's -e option and bail out asap if any command goes bad. Jenkins CI uses this. And I think deploying scripts are even more critical than CI scripts.

Hope it gets merged :)

@j15e
Copy link

j15e commented Nov 29, 2013

+1

1 similar comment
@rpocklin
Copy link

+1

@jeremy
Copy link

jeremy commented Feb 15, 2014

Seems this should be in the bash template: https://github.com/nadarei/mina/blob/master/data/deploy.sh.erb

Rather than inserted in the ssh helper.

@Kriechi
Copy link

Kriechi commented Aug 3, 2014

Good idea - but it should go into the bash template as mentioned by @jeremy.
@5long could you change that, maybe then this PR will get merged?

@5long
Copy link
Contributor Author

5long commented Jan 2, 2015

Hell, I haven't been doing programming things for a long time and I'm recovering from it.

Since the previous patch is long outdated I just did a new commit and did a forced update to my branch w/ the -e option put in the correct file.

d4be4st added a commit that referenced this pull request Jul 5, 2015
Use `set -e` to fail fast
@d4be4st d4be4st merged commit de4e3fa into mina-deploy:master Jul 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants