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

accepting location as an input parameter #4

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

Conversation

ajaygtm71
Copy link

Solves #3

@Cazaimi Cazaimi self-requested a review December 28, 2020 09:17
Copy link
Owner

@Cazaimi Cazaimi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @ajaygtm71! Left comments.

# Boot up github shell access for Windows WSL/Ubuntu.

#!/bin/bash
if [ "$#" -lt 1 ]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajaygtm71 , we want to make the location parameter optional here, and not mandatory. Probably behind a flag such as -l <location> or --location <location>.


private_key_location=~/.ssh/github_rsa # Change this location to where your file is located.
myArray=( "$@" )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs a better name.


private_key_location=~/.ssh/github_rsa # Change this location to where your file is located.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't remove this yet, we can have a fallback.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you want to keep both ways ie manually editing and optionally passing parameters?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. As mentioned above, the location parameter is optional.

echo 'Booting up ssh-agent...'
fi

eval `ssh-agent -s`
# ssh_agent_running=$(ssh-agent -s)
ssh_agent_running=$(ssh-agent -s)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you test this once? I reckon that this might not work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the variable is storing the command output.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but the ssh-agent -s command actually outputs the ssh-agent's pid and the socket location and running the command inside eval stores them as local variables. If the eval is removed, ssh-add will no longer work, as shown below:

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

echo 'ssh-agent started with output:' $ssh_agent_running
echo 'Adding private key to ssh-agent...'
fi

ssh_add_result=$(ssh-add $private_key_location)

if [ "$1" = "-v" ]; then
echo 'ssh-agent started with output:' $ssh_agent_running
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The status messages are a part of the 'verbosity' 😅

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same msg is already there in previous if block. so I removed from here


private_key_location=~/.ssh/github_rsa # Change this location to where your file is located.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. As mentioned above, the location parameter is optional.

echo 'Booting up ssh-agent...'
fi

eval `ssh-agent -s`
# ssh_agent_running=$(ssh-agent -s)
ssh_agent_running=$(ssh-agent -s)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but the ssh-agent -s command actually outputs the ssh-agent's pid and the socket location and running the command inside eval stores them as local variables. If the eval is removed, ssh-add will no longer work, as shown below:

echo 'Booting up ssh-agent...'
fi

eval `ssh-agent -s`
# ssh_agent_running=$(ssh-agent -s)
ssh_agent_running=$(ssh-agent -s)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

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.

2 participants