Skip to content
This repository has been archived by the owner on Dec 1, 2018. It is now read-only.

Fix raw container stats retrieval from kubelet over https #636

Merged
merged 1 commit into from
Oct 9, 2015

Conversation

jimmidyson
Copy link
Contributor

@jimmidyson
Copy link
Contributor Author

@mvdan @vishh Would love a quick review of this if possible - trivial fix with big impact to us :(

@mvdan
Copy link
Contributor

mvdan commented Oct 9, 2015

LGTM. Nit though - it's usually preferred to define the default value then override it with special cases, like:

scheme := "http"
if self.config != nil && self.config.EnableHttps {
    scheme = "https"
}

@jimmidyson
Copy link
Contributor Author

@mvdan Much nicer. Updated.

@jimmidyson
Copy link
Contributor Author

@mvdan OK to merge?

mvdan added a commit that referenced this pull request Oct 9, 2015
Fix raw container stats retrieval from kubelet over https
@mvdan mvdan merged commit 0c05897 into kubernetes-retired:master Oct 9, 2015
@mvdan
Copy link
Contributor

mvdan commented Oct 9, 2015

Sure :) I thought you would self-merge since I gave my LGTM.

@jimmidyson
Copy link
Contributor Author

@mvdan I don't know the rules here yet - each community has different rules. But I know for next time - thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants