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

restore support for env variables to configure proxy #2059

Merged
merged 1 commit into from Aug 22, 2019

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Aug 22, 2019

regression introduced by b34f340 (#1501)

fixes #2027
fixes moby/moby#39654

Signed-off-by: Nicolas De Loof nicolas.deloof@gmail.com

- What I did
fix moby/moby#39654

- How I did it
restored order of client.Opt. WithHttpClient must come first

- How to verify it
added a test case to cover this scenario

- Description for the changelog
restore support for env variables to configure proxy

- A picture of a cute animal (not mandatory but encouraged)
image

regression introduced by b34f34
close #39654

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @ndeloof !! 👍

@codecov-io
Copy link

Codecov Report

Merging #2059 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2059   +/-   ##
=======================================
  Coverage   56.78%   56.78%           
=======================================
  Files         311      311           
  Lines       21836    21836           
=======================================
  Hits        12400    12400           
  Misses       8520     8520           
  Partials      916      916

1 similar comment
@codecov-io
Copy link

Codecov Report

Merging #2059 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2059   +/-   ##
=======================================
  Coverage   56.78%   56.78%           
=======================================
  Files         311      311           
  Lines       21836    21836           
=======================================
  Hits        12400    12400           
  Misses       8520     8520           
  Partials      916      916

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

nice find; easy to miss!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants