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

Prepare train for using credential sets #394

Merged
merged 5 commits into from
Jan 31, 2019
Merged

Conversation

clintoncwolfe
Copy link
Contributor

Train has historically either used explicit options passed in a hash, or parsed a --target option as a URL. Both are a bit unwieldy, especially the URI method.

This PR makes a few changes to make it easier to work with credentials passed in as a hash.

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
lib/train.rb Show resolved Hide resolved
lib/train.rb Show resolved Hide resolved
Copy link
Contributor Author

@clintoncwolfe clintoncwolfe left a comment

Choose a reason for hiding this comment

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

software engineer, debug thyself!

lib/train.rb Show resolved Hide resolved
lib/train.rb Outdated Show resolved Hide resolved
lib/train.rb Outdated Show resolved Hide resolved
test/unit/train_test.rb Outdated Show resolved Hide resolved
@@ -228,7 +229,7 @@
end

it 'returns the local backend if nothing was provided' do
Train.validate_backend({}).must_equal :local
Train.validate_backend({}).must_equal 'local'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If all backend driver names are strings, how did this ever pass as symbols?

lib/train.rb Outdated Show resolved Hide resolved
lib/train.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@marcparadise marcparadise left a comment

Choose a reason for hiding this comment

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

Looks good once comments addressed

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Copy link
Contributor

@jerryaldrichiii jerryaldrichiii 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 this @clintoncwolfe!

@@ -3,5 +3,6 @@
require 'minitest/autorun'
require 'minitest/spec'
require 'mocha/setup'
require 'byebug'
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey! It's in the helper now 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aspect: Integration Type: Enhancement Improves an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants