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

feat: Implemented default values for undefined env vars #766

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kael-shipman
Copy link

No description provided.

closes db-migrate#765

Signed-off-by: Kael Shipman <kael.shipman@gmail.com>
@kael-shipman kael-shipman changed the title Implemented default values for undefined env vars feat: Implemented default values for undefined env vars Jan 21, 2022
@kael-shipman
Copy link
Author

@wzrdtales I'm not sure what to do to please the Semantic PR rule.... I've tried a number of edits to my commit and to the PR title, but none seem to have worked.

Also, I know we didn't really talk about this before I implemented, but I figured I'd take the risk.... lemme know if you see any issues with this feature.

@kael-shipman
Copy link
Author

@wzrdtales anything wrong with merging this?

@wzrdtales
Copy link
Member

I don't get your PR. What is it suppose to add? A default you have to configure is not a default. And why would you place a "default" value, instead of just providing the actual config?

@kael-shipman
Copy link
Author

Without this feature, if you want to be able to use environment variables for configuration, then you must set all variables. This change allows you to configure default values that are used when a given environment variable is not set, which can be useful if, for example, you have a dev setup that uses mostly common values, but individual devs may want to make minor adjustments.

I can certainly understand that some people would not be interested in this, but of course they also don't have to use it or think about it. it's a change that enables functionality for people who want it without degrading current functionality in any way.

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.

None yet

2 participants