Add support for configuration files. #19
Conversation
matro
force-pushed
the
feature/env-files
branch
2 times, most recently
from
e544903 to
938f8f1
Compare
|
I've only covered "the big three" vars here, but I think other things like |
|
I'm also not sure how to write tests for this stuff, if that's something we need here. |
| repo=${GIT_DEPLOY_REPO:-origin} | ||
|
|
||
| #append commit hash to the end of message by default | ||
| append_hash=true |
There was a problem hiding this comment.
as this is currently written, the append_hash default will need to go before the arg parsing, so it doesn't override it.
|
i would like to see the order of preference tested for one of the variables, but i'm not going to say it's required right now, since the rest of the tests are still a work in progress. for such a test, i would recommend splitting the arg parsing into a separate function, so it can be tested in isolation. |
| @@ -29,10 +24,24 @@ main() { | |||
| elif [[ $1 = "-n" || $1 = "--no-hash" ]]; then | |||
| append_hash=false | |||
| shift | |||
| elif [[ $1 = "-c" || $1 = "--config-file" ]]; then | |||
There was a problem hiding this comment.
Thought about it a little and... should we check that $2 is defined (as is done with -m)? Or is an error like ./deploy.sh: line 28: <string>: No such file or directory good enough?
There was a problem hiding this comment.
that's good enough for me. i think it's better to give some kind of error than silently ignore a missing parameter.
Doesn't work if it's set in `config-file`. This reverts commit 5f38074.
|
This is turning out to be more squirrelly than I was expecting. I think I'm going to keep poking at this a bit, and try to get it covered by tests of some kind before putting up any more commits here. I've got another branch where I've started taking a stab at the positional arguments. Was considering sticking that work in here, but I think I'll keep them separate; hopefully get that done without being blocked my my whacky cascade o' config-files idea. |
This brings back commit 5f38074 except it actually works this time. It also establishes a pattern of "if it's an env-var, overwrite _that_ when parsing args, then set the internal var using the env-var/defaults".
This should allow a config. file specified on the command-line to
overwrite values set in '.env', which in turn ovewrite values set
in the environment. Values set for these same vars in the args-
parsing loop should then overwrite all of the above.
Anything left unset by environment vars above are then covered when
the "internal" vars are populated by the `var=${ENV_VAR:-default}`
lines.
|
I've got some tests added, including a failing one for the append_hash setting. |
matro
force-pushed
the
feature/env-files
branch
from
1e2041f to
68a726c
Compare
matro
force-pushed
the
feature/env-files
branch
from
2de4a1c to
a1e762a
Compare
|
Now using a temporary directory (and wow, And I think the cascade of var-setting works for reals now, with test coverage. It was starting to look like I had a typo somewhere, but clearing out my first stab at the tests and starting over helped a lot. Also follows the whitespace/language style that's in the other tests. These tests are in one file now, and are back in the project root. I'm pretty sure the setup/teardown and setting-setter functions here will work for testing #20 as well... though I guess I'll see when I get there. |
| set_env_vars() { | ||
| # Set environment variables. | ||
| export GIT_DEPLOY_USERNAME=env-username | ||
| export GIT_DEPLOY_EMAIL=env-email |
There was a problem hiding this comment.
did you have a plan for these email and username variables? Otherwise, looks great! I'll merge now.
There was a problem hiding this comment.
I have it in my head to set those things using git config info, but I haven't gotten to that yet.
I was using them to help me figure out why my tests for append_hash were failing as well, and neglected to remove them when I got things working. No need to keep them in here, I guess. I'll just re-add them if/when I need them later.
|
Since this PR is still open, I'm going to sneak one more test in here. Looks like whatever test I wrote the first time around for |
matro
force-pushed
the
feature/env-files
branch
from
9e55543 to
b750ac3
Compare
.envexists in the current path, it gets sourced.-c $FILEor--config-file $FILEis specified,$FILEgets sourced.Values set earlier get overriden by those set later.
Fixes part of #14.