-
Notifications
You must be signed in to change notification settings - Fork 492
Add User-Agent to HTTP notifications #1097
Conversation
|
This change only covers adding the The |
|
We already set http.DefaultClient to a custom client here: Line 34 in 080bbf1
Any changes should be made there. Adding a "ShortVersion()" function to the version package would be ok by me as well. |
|
@captncraig ah, great, I'll use I'll work on the |
|
I would think we should set up out main client to set the user-agent on every request. That can be done with a custom RoundTripper. Something like this: https://play.golang.org/p/_E6fRYPvrN |
605ff23 to
02b1d03
Compare
|
@captncraig I've applied your suggestions. I think the initialization of the client could probably be done in a nicer way than this. I'd like to know your thoughts. |
|
LGTM. How do you feel about this implementation @mjibson ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not set this if it already exists.
|
Implementation looks good after my one comment. What problem does this solve, or why is it needed? |
It isn't really a problem what is solving, maybe a best practice. At work we're working on a custom notifications aggregator, which several other services sending HTTP notifications. We record all the information received and use the I'll see to fix that line you pointed me, sorry for the delay. Thanks! |
|
Closing. Will merge commit in #1265 |
|
Thanks! |
Use a custom
http.Clientfor sending HTTP notifications, adding a properUser-Agentheader with theBosun/{{version}}format. The version only includes the version number and release tag, not the most recent commit hash nor build time.