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

Change *http.Transport to http.RoundTripper #123

Merged
merged 1 commit into from Feb 24, 2019
Merged

Change *http.Transport to http.RoundTripper #123

merged 1 commit into from Feb 24, 2019

Conversation

radeksimko
Copy link
Contributor

https://golang.org/pkg/net/http/#RoundTripper

This will allow anyone to pass through custom transport implementations, such as logging transport we use over in Terraform:

https://github.com/hashicorp/terraform/blob/master/helper/logging/transport.go

*http.Transport implements this interface so this should not break anything.

I tried running tests via make test but they mostly fail (on master) for me. It may be just different environment, I don't know - I hope my changes didn't break anything. 🤞

@michaelklishin
Copy link
Owner

@radeksimko there are notes on how to set up a RabbitMQ node for running this client's tests. I'm happy to run the suite on your behalf.

I suspect we need to bump major version to go with this change?

@michaelklishin
Copy link
Owner

The test suite is happy:

=== RUN   TestRabbitHole
Running Suite: Rabbithole Suite
===============================
Random Seed: 1551039226
Will run 66 of 66 specs

••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••
Ran 66 of 66 Specs in 67.267 seconds
SUCCESS! -- 66 Passed | 0 Failed | 0 Pending | 0 Skipped

@michaelklishin michaelklishin added this to the 2.0.0 milestone Feb 24, 2019
@michaelklishin michaelklishin merged commit 20d7ce3 into michaelklishin:master Feb 24, 2019
@michaelklishin
Copy link
Owner

Thank you!

@radeksimko radeksimko deleted the change-iface branch February 24, 2019 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants