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

Don't set the transport's proxy settings to nil by default #39

Closed
neilotoole opened this issue Sep 30, 2016 · 2 comments
Closed

Don't set the transport's proxy settings to nil by default #39

neilotoole opened this issue Sep 30, 2016 · 2 comments
Assignees

Comments

@neilotoole
Copy link

In client.go, at this location, there's this code:

    if req.proxyURL != nil {
        c.transport.Proxy = http.ProxyURL(req.proxyURL)
    } else if c.proxyURL != nil {
        c.transport.Proxy = http.ProxyURL(c.proxyURL)
    } else {
        c.transport.Proxy = nil
    }

The final else clause should be removed. If the caller provides a transport, it may already have a proxy configured, there's no reason to set the transport's proxy to nil.

@neilotoole neilotoole changed the title Don't override the transport's proxy settings Don't set the transport's proxy settings to nil by default Sep 30, 2016
@jeevatkm jeevatkm self-assigned this Sep 30, 2016
@jeevatkm jeevatkm added this to the v0.9 Milestone milestone Sep 30, 2016
@jeevatkm
Copy link
Member

Make sense. Yes, we can leave that to caller.

Would you like to send PR?

@jeevatkm
Copy link
Member

jeevatkm commented Oct 2, 2016

Taken care.

@jeevatkm jeevatkm closed this as completed Oct 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants