Skip to content

Conversation

@runcom
Copy link
Member

@runcom runcom commented Jul 18, 2016

And avoid creating 3 clients in docker_client.go

@mtrmac PTAL

Signed-off-by: Antonio Murdaca runcom@redhat.com

@runcom
Copy link
Member Author

runcom commented Jul 18, 2016

lgtm ping @mtrmac

Approved with PullApprove

httpClient = &http.Client{Transport: transport}
httpClient.Transport = transport
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

While openshift-copies.go is modified from the originals, I’d prefer not introducing new behavior in there. Could this be moved to openshift.go (which contains the only caller of this function)?

Copy link
Member Author

Choose a reason for hiding this comment

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

mm sure, I don't understand what I actually have to move

Copy link
Collaborator

@mtrmac mtrmac Jul 18, 2016

Choose a reason for hiding this comment

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

  • Do not change openshift-copies.go
  • In openshift.go:newOpenshiftClient, set the Timeout as desired.

@mtrmac
Copy link
Collaborator

mtrmac commented Jul 18, 2016

ACK on the docker part; can openshift be moved?

@runcom runcom force-pushed the refa branch 2 times, most recently from b163a94 to 29d43b7 Compare July 18, 2016 14:37
@runcom
Copy link
Member Author

runcom commented Jul 18, 2016

@mtrmac rebased good to go after your LGTM

if *baseURL != *ref.baseURL {
return nil, fmt.Errorf("Unexpected baseURL mismatch: default %#v, reference %#v", *baseURL, *ref.baseURL)
}
http.Client.Timeout = 1 * time.Minute
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fails build; s/http.Client/httpClient/?

Copy link
Member Author

Choose a reason for hiding this comment

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

my bad, fixed

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@mtrmac
Copy link
Collaborator

mtrmac commented Jul 18, 2016

👍

Approved with PullApprove

@mtrmac mtrmac merged commit 2cfa81f into containers:master Jul 18, 2016
@runcom runcom deleted the refa branch July 18, 2016 15:41
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.

2 participants