search

autopilotpattern / couchbase

16 stars 9 forks source link

Add COUCHBASE_SERVICES option and fix non-linted (shellcheck) code #19

Closed konobi closed 8 years ago

konobi commented 8 years ago

COUCHBASE_SERVICES now able to be passed through as an env variable and includes validation for the service types.

I also did a lint check on the bash scripts and fixed some reported warnings from shellcheck.

Also ensured that image setup could be run against a local docker installation as well as Triton.

konobi commented 8 years ago

I updated so the default works with local docker installs. I've been trying to test the triton setup, but following the documentation as is, I haven't been able to get triton profile get to work =0(

tgross commented 8 years ago

I also did a lint check on the bash scripts and fixed some reported warnings from shellcheck.

Can you undo those changes please and submit them under separate PR if you think they're important? I don't want to have to detangle the actual changes from style changes in order to review this. I'm pretty happy with the style FWIW. The "declare and assign" thing is begging for bugs.

konobi commented 8 years ago

shellcheck isn't so much a style checker as much as it's a linter for ensuring that the shell code is doing what you expect, since there's lots of subtleties that aren't obvious.

You can run shellcheck on the current code and get a list of problems, each of which have a specific SC---- number, which you can then look at in the shellcheck wiki to get a full breakdown on why it's making that suggestion, other options and how to write the same thing without the side effects described. It's homebrew installable, so it's nice 'n quick =0)

misterbisson commented 8 years ago

@konobi are you going to update this PR to address the feature request without the linting changes?

konobi commented 8 years ago

yup... I've added it as a new PR.