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

support https and pluggable dialers #20

Closed
wants to merge 2 commits into from
Closed

support https and pluggable dialers #20

wants to merge 2 commits into from

Conversation

jtolio
Copy link

@jtolio jtolio commented Jan 24, 2014

closes #18 and #8

i'm unhappy that Dial matches the signature of net.Dial. :(

accepting network/address to the Dialer interface that does caching seems like the wrong abstraction layer here though, since we want to pool connections based on if they're ssl or not.

so, possible options: rename Dial to something else, Connect maybe? or maybe make some Scheme and Host types that are just strings?

i dunno.

@zeebo
Copy link

zeebo commented Jan 25, 2014

This looks good to me but I'm not familiar with the code base/design decisions very well. @davecheney ?

@lalloni
Copy link

lalloni commented Nov 11, 2014

How would I use the result of this patch to setup an http(s) client with a specific tls.Config provided?

@jtolio
Copy link
Author

jtolio commented Nov 11, 2014

oh whoa old pull request. i'm not sure i even want to merge this anymore

@lalloni
Copy link

lalloni commented Nov 12, 2014

Meaning there's some other way of using gorilla/http over ssl/tls?

@jtolio
Copy link
Author

jtolio commented Nov 13, 2014

oh, i don't actually know the answer to that. we stopped using gorilla/http, but i definitely have some complaints with this patchset as-is. i was kind of a go-newbie

@lalloni
Copy link

lalloni commented Nov 16, 2014

go-newbie, exactly, that's me now...

can I ask why you walked away from gorilla/http? what you changed it for?

@jtolio
Copy link
Author

jtolio commented Jan 3, 2015

we're just using the standard library net/http

@kisielk kisielk closed this Aug 13, 2015
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.

Example doesn't work with https://api.github.com
4 participants