Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

vpc #7

merged 2 commits into from Jun 1, 2012


None yet
2 participants

aaronfeng commented May 31, 2012

ability to connect to nodes within vpc.

This is great!


aaronfeng replied Jun 1, 2012

I noticed that you received another pull request in the same area. Want me to resolve it with my changes?

I already started to so I'll merge it


aaronfeng replied Jun 1, 2012

lol, just finished about to push..

social coding FTW

Merge branch 'master' of https://github.com/wbailey/claws

aaronfeng commented Jun 1, 2012

I feel like I'm racing with you :) Not sure how to handle multiple pull requests when there's one pending, but here is the change aaronfeng/claws@3941a84

wbailey commented on 3941a84 Jun 1, 2012

Why the refactor to an ssh method that doesn't add any clarity? I would have preferred to see something like:

identity = config.ssh.identity.nil? ? '' : "-i #{config.ssh.identity}"
current_instance = instances[selection.to_i]
use_address = current_instance.vpc? ? current_instance.private_ip_address : current_instance.public_dns

system "ssh #{identity} #{config.ssh.user}@#{use_address}"

aaronfeng replied Jun 1, 2012

The new code isn't more clear than the old. The goal was to provide a simple abstraction layer in order to remove some of the duplications. I think we are talking mostly about style here. I'm down with whatever.

wbailey added a commit that referenced this pull request Jun 1, 2012

@wbailey wbailey merged commit eb2ccbe into wbailey:master Jun 1, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment