configuration with command-line arguments

[X1011]

I'm thinking about changing how the configuration is specified. Rather than having users modify the script or set environment variables, the script would take additional command-line arguments. I would do this for deploy_directory, deploy_branch, and repo. I would eliminate default_username and default_email and just require the user to have git sufficiently configured beforehand.

I'm considering a syntax like this: deploy.sh [<options>] [<directory> [<branch> [<repository>​]]]

pros:

• the script doesn't need to be modified, so it can be shared across projects, installed as a package, and write protected
• no polluting of environment variables, no chance of name collisions
• makes apparent that a relative deploy_directory is relative to the current working directory, eliminating the need to mention this in the readme

cons:

• complicates ad-hoc deployments not using the defaults, as you would now have to either remember to include the arguments every time or create a wrapper script
• having 3 positional arguments, it may be hard to remember which is which
• will probably complicate argument parsing

questions:

• which arguments should be positional or named? (e.g., should we do something like this instead?: deploy.sh [-d <directory>] ...)
• which arguments should still have defaults or be required?

Let me know if you think this is a good or bad idea.

[LeonardDrs]

Hi X1011.

I recently came across your script and it suits me -almost- well.
I like flexibility and think this enhancement is a good idea since I added my own code to yours in order to use additional command-line arguments.

My syntax is currently like this deploy.sh [<directory> [<branch> [<repository>​]] [<options>]

About your cons:

• Agree, but actually I'm using a wrapper as I need to do others things
• We could use a deploy.hs -h to display the syntax.
• Yeah it'll.

About your questions:

• I kinda like it when arguments are positional, but it's up to you.
• My guess would be using default values for the 3 arguments. dist, gh-pagesand origin.

Cheers

[matro]

I think this is a fantastic idea! I'd actually started writing a fork of this script to remove configuration bits (and do some other stuff that probably doesn't matter), but this makes me want to drop my fork and come back to this project.

I agree with @LeonardDrs about the defaults for those three arguments; they're totally sane.

[<directory> [<branch> [<repository>​]] order makes sense to me too. I don't have a strong opinion whether [<options>] should come before or after the positional stuff, though I tend to prefer tools that work either way (see rsync). It'd make args-parsing more complicated, but usage more flexible and friendly to human foibles.

Here're a couple of ideas that might make things even more flexible, robust, and mitigate some of the cons for ad-hoc usage:

1. Add sourceing a .env file to set variables, and a --config option to specify an alternate file name.
   • This'd enable "project-local" environment variables, and avoid polluting actual environment variables.
   • There is a bunch of precedence for doing this.
2. Set variables in this logical order, where whatever the most "specific" setting (higher number in this list) wins:
   1. Script's default (yay for assuming nothing!)
   2. Environment variables (support current assumptions about env vars)
   3. .env file (or the vars-file specified on the command-line) if this was implemented.
   4. Command-line's [<directory> [<branch> [<repository>​]] strings.

This'd let users run the script with the existing "the script's defaults work for me" and "my environment vars are fine" assumptions, plus let "project-specific" environments and ad-hoc command-line'ing override/replace the more generally-scoped vars.

Would totally make sense to punt "add support for .env files" to another issue (and I'd be happy to try implementing it)... but I figured it'd make sense bringing up here first.

[X1011]

@matro, cool, that all sounds good. Have at it!

[matro]

I think there are pull requests for all three aspects of this now.

[matro]

I'm game for trying to get the args-parsing into its own function with test coverage as well... should we take that to a separate issue/PR, or keep this thread going?

[X1011]

I'm gonna say keep this thread going, as all the PRs are in support of this goal. Then, the last PR out can close this issue.

[matro]

So I've learned a bit about writing bats tests, and have some new test files added to both PRs. I put them in tests/ to try to keep the project root clean, but haven't done anything to existing test files or the watch script yet.

Should I pop my new test files up to the project root, or would it make sense to put all tests in the tests folder?

[X1011]

I think you have too many tests now. Most of the variables are being parsed the exact same way, so the tests aren't testing unique behaviors, and the extra tests aren't adding much value, but they complicate maintenance and onboarding, increase run time, etc.
I recommend:

• 5 tests for append_hash, checking that it gets overridden for each progressively higher-priority config source
• a test for --message, since it seems you've found an issue with messages containing spaces, and
• whatever tests are needed for the positional args.

The rest i think we can do without.

That may change how you want to organize the tests into files, but I'm gonna say once we end up with 4 or more .bats test files in the root, then let's put them in their own folder. And don't forget to update circle.yml so that the CI server runs them.

Also, i don't know if you've noticed the 'pseudo groups' i've created using clever whitespace in the test names. I was going for an RSpec/Mocha-style format. What do you think, is it worth continuing that style?

[matro]

Okay, I'll pair down and consolidate things a bit. This is my first time actually writing unit tests of any kind, so I'm kind of learning as I go. I was also trying to tease out any more weird cases like append_hash; I don't trust that my mental model matches the code yet.

I did notice the 'pseudo groups' thing you did. I think it's definitely worth continuing. I haven't tried to follow it so far because I'm not how to logically group this stuff yet. Definitely makes more legible output.