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

Don't Repeat Yourself #6

Open
johnwyles opened this issue Oct 23, 2018 · 4 comments
Open

Don't Repeat Yourself #6

johnwyles opened this issue Oct 23, 2018 · 4 comments

Comments

@johnwyles
Copy link

johnwyles commented Oct 23, 2018

Is your feature request related to a problem? Please describe.
Repeating yourself

Describe the solution you'd like
Pull the configuration into a single file

Describe alternatives you've considered
N/A

Additional context
N/A

You have this atop all of your scripts and if this ever changes, for whatever reason (which it probably won't), you'll be copy/pasting it all over again throughout. Also if someone wants to add some initialization steps beyond what is specified here you may benefit them as they could simply add to this included file. So instead of this:

if ! grep -q aws_access_key_id ~/.aws/config; then
  if ! grep -q aws_access_key_id ~/.aws/credentials; then
    echo "AWS config not found. Running \"aws configure\"."
    exit 1
  fi
fi

Move it to _pre_command.sh and improve it:

#!/usr/bin/env bash

# Look for the AWS CLI
if test ! $(which aws); then
  echo "Installing AWS CLI..."
  brew install awscli
fi

# Look for the AWS configuration
if ! grep -q aws_access_key_id ~/.aws/config; then
  if ! grep -q aws_access_key_id ~/.aws/credentials; then
    echo 'AWS configuration was not found. Running "aws configure"...'
    aws configure
  fi
fi

Then in all of your scripts simply put:

$SWOODFORD_AWS_DIRECTORY/_pre_command.sh

Or something more fancy (but that I think isn't really necessary):

$SWOODFORD_AWS_DIRECTORY/_pre_command.sh || sh -c 'echo; echo "The file \"\$SWOODFORD_AWS_DIRECTORY/_pre_command.sh\" was not found or executable! Please see this page for help: https://github.com/swoodford/aws"'

And then you can put this in an install.sh script:

#!/usr/bin/env bash

# Look for Homebrew and install it if not found
if test ! $(which brew); then
  echo "Installing homebrew..."
  /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
fi

# Get the code
git clone https://github.com/swoodford/aws.git ~/.swoodford_aws

# Set this (probably not needed)
export SWOODFORD_AWS_DIRECTORY=~/.swoodford_aws

# Add SWOODFORD_AWS_DIRECTORY variable for reference
grep -q -i -F 'SWOODFORD_AWS_DIRECTORY' ~/.bash_profile || sh -c 'echo "SWOODFORD_AWS_DIRECTORY=\"~/.swoodford_aws"\" >> ~/.bash_profile'

# Add SWOODFORD_AWS_DIRECTORY to the PATH
grep -q -i -F 'PATH="~/.swoodford_aws' ~/.bash_profile || sh -c 'echo "PATH=\"~/.swoodford_aws:\$PATH\"" >> ~/.bash_profile'

Then you can tell users "To simply install these scripts run the following:
sh -c "$(curl -fsSL https://raw.githubusercontent.com/swoodford/aws/master/install.sh)" > /dev/null"

@johnwyles
Copy link
Author

You can also move all of the functions to this _pre_command.sh:

# Check Command
function check_command {
	type -P $1 &>/dev/null || fail "Unable to find $1, please install it and run this script again."
}

# Completed
function completed(){
	echo
	HorizontalRule
	tput setaf 2; echo "Completed!" && tput sgr0
	HorizontalRule
	echo
}

# Fail
function fail(){
	tput setaf 1; echo "Failure: $*" && tput sgr0
	exit 1
}

# Horizontal Rule
function HorizontalRule(){
	echo "============================================================"
}

# Pause
function pause(){
	read -n 1 -s -p "Press any key to continue..."
	echo
}

And then instead of the line I have above you would simply add a dot in front of it:

. $SWOODFORD_AWS_DIRECTORY/_pre_command.sh

@swoodford
Copy link
Owner

Hey @johnwyles yes I agree there is room for improvement here.

These scripts have been developed over a span of several years and some of them follow different patterns and could use some cleanup. I considered moving to a single script that handled all the common functions, but my concern is if someone is only interested in one tool and they wanted to run just that without cloning the whole repo and it is missing some function from another script.

I'll have to review all the scripts and see which ones need updating then come up with a good way to support this without breaking anything in the process!

@johnwyles
Copy link
Author

@swoodford it's been nearly a year now - I am wondering if you should close this or if I can help address anything further as I have now some bandwidth opening up to assist

@gchamon
Copy link

gchamon commented Jan 31, 2020

it should be interesting to document dependencies. I for instance use a single script, downloaded from a beanstalk ec2 instance. If it had dependencies, just downloading this single script wouldn't be enough.

Either document dependencies or clearly specify that there are indeed dependencies within the repository, so we are aware of that and instead of downloading single files, we clone the entire repo.

Repository owner deleted a comment from DavideStagni Mar 19, 2024
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

No branches or pull requests

3 participants