-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Added version check for bash version gteq 4 #292
Added version check for bash version gteq 4 #292
Conversation
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.
Haven't directly tested this but had a play with ${BASH_VERSINFO}
in my shell (didn't know it was a thing!) and it looks like this should work well, thanks for the fix @jmfrank63
I think adding a note to the changelog would be good if you have a moment to do so?
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.
LGTM 👍
Please add an entry to the changelog and I'll get this merged.
CHANGELOG.md
Outdated
@@ -232,3 +232,4 @@ v2.2 as released by Bitly. | |||
- Repository forked on 27/11/18 | |||
- README updated to include note that this repository is forked | |||
- CHANGLOG created to track changes to repository from original fork | |||
- Added bash >= 4.0 dependency to configure script |
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.
Sorry, this should be of the format:
- [#292](https://github.com/pusher/oauth2_proxy/pull/292) Added bash >= 4.0 dependency to configure script (@jmfrank63)
And should be at the top of this file with the more recent changes (changes since v4.0.0)
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.
Thank you and congrats on your first contribution! 🎉
Description
Fixes #291
Motivation and Context
How Has This Been Tested?
I ran configure against bash version <4. The script fails with the reported error.
I then upgraded to bash version 5 via homebrew.
As a result the script ran fine.
I uninstalled bash again falling back to the native OSX version.
I ran configure again resulting in:
This script requires Bash version >= 4
After reinstalling bash via homebrew again the script did work again.
Checklist:
I am not yet fully confident of the style on a CHANGELOG entry. I am willing to do the change but would like some guidance.