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

Export BuildID via expvars. #1189

Merged
merged 1 commit into from
Nov 24, 2015
Merged

Export BuildID via expvars. #1189

merged 1 commit into from
Nov 24, 2015

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Nov 24, 2015

Fixes #1188

@jsha jsha added the r? label Nov 24, 2015
@@ -21,6 +21,7 @@ import (
"encoding/json"
"encoding/pem"
"errors"
"expvar"
Copy link
Contributor

Choose a reason for hiding this comment

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

So, packages using core will get /debug/vars by virtue of this import.

Maybe we should be explicit about importing expvar in cmd where we set up the other debug handlers?

It'd be a _ "expvar" import there.

@jmhodges
Copy link
Contributor

LGTM but that one small additional import in cmd could be done, too, and might be nice for clarity and future-proofing against a possible break-up of the core package.

@jmhodges
Copy link
Contributor

AMQP Timeout here.

I'm into how much more common they are becoming. Think we might have pushed these things far enough that we can repro them reliably enough to actually debug them in TravisCI.

@jcjones
Copy link
Contributor

jcjones commented Nov 24, 2015

r=jcjones

@jsha
Copy link
Contributor Author

jsha commented Nov 24, 2015

Re-pushed with import _ "expvar". Re-review @jmhodges @jcjones?

@jcjones
Copy link
Contributor

jcjones commented Nov 24, 2015

r=jcjones (re-r)

@jmhodges
Copy link
Contributor

LGTM

jmhodges added a commit that referenced this pull request Nov 24, 2015
Export BuildID via expvars.
@jmhodges jmhodges merged commit 9776db7 into master Nov 24, 2015
@jmhodges jmhodges deleted the expvar branch November 24, 2015 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants