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

Defer errexit to as late as possible #70

Closed

Conversation

apeschel
Copy link

At the moment, transcrypt immediately exits with a failure when run
outside of a git repository, regardless of what command is given to
transcrypt. This causes unexpected behavior when running commands like
"help" or "version".

This commit moves errexit to as late as possible, while still keeping it
as a safe guard for the time being. Ideally, errexit would not be used
in transcrypt at all.

At the moment, transcrypt immediately exits with a failure when run
outside of a git repository, regardless of what command is given to
transcrypt. This causes unexpected behavior when running commands like
"help" or "version".

This commit moves errexit to as late as possible, while still keeping it
as a safe guard for the time being. Ideally, errexit would not be used
in transcrypt at all.
@apeschel
Copy link
Author

Fixes #68

@apeschel
Copy link
Author

@elasticdog Could you weigh in on this when possible, please?

@elasticdog
Copy link
Owner

Hey @apeschel! Thank you for the PR, and my apologies on taking so long to respond. I think I've come up with a cleaner solution to issue #68 without having to shuffle around the errexit timing. There were a lot of changes to enable "strict" Bash within #53 that took me a while to warm up to, but now I can see the value (especially since transcrypt doesn't have a test suite we can rely on, it can help when adding new features).

Let me know if my other PR addresses the issues you were tackling here, and I'd also be curious to hear your thoughts on why you think errexit should be eliminated completely. Cheers.

@elasticdog
Copy link
Owner

I'm going to close this one out, as the original issue was addressed via #71, but thanks again for taking the time to look at the problem...I'd be glad to discuss errexit with you further if you have strong feelings about it.

@elasticdog elasticdog closed this Dec 9, 2019
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.

2 participants