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

Implemented local override config file #39

Merged
merged 2 commits into from
Jun 14, 2018
Merged

Conversation

kael-shipman
Copy link
Contributor

This PR allows users to specify a local configuration file shmig.local.conf that they can ignore from version control, such that they may specify instance-specific configs that override the default configs in shmig.conf.

This is a scheme I much prefer to environment variables, as there is virtually no devops overhead -- you just add the configs in the local file and any user that runs the program will have access to them.

This can be used to easily implement multiple environments, as you've demonstrated with prod vs test, among other things.

Also, if you haven't seen bash_unit, I've found it to be pretty cool. I didn't use it here because I didn't want to disrupt your test flow, but thought you might like to know :).

shmig Outdated
# any error in configuration file should cause failure
# to avoid potential database corruption
trap 'die "Local configuration failed"' ERR
source "$LOCAL_CONFIG"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like source is a bashism (https://stackoverflow.com/a/11588636/1789168)

Please use . (dot) as that is POSIX.

@mbucc
Copy link
Owner

mbucc commented Jun 13, 2018

What a great PR, thanks for you efforts! I suggested one small tweak to drop a bashism; see the comment I made in the PR diff.

@kael-shipman
Copy link
Contributor Author

Haha, that's funny, I actually only used source because it was used in the original config function on line 649. I've changed both instances to '.'.

@mbucc mbucc merged commit cc25779 into mbucc:master Jun 14, 2018
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