Adding the "user-agent" Header #3720
Conversation
DarthSett
requested review from
bboreham,
fbarl,
qiell and
satyamz
as code owners
|
Thanks for the PR! |
|
I changed it to "Probe_Scope/probeversion". Please review the changes. I have committed and pushed them to my fork. |
There was a problem hiding this comment.
Thanks for your contribution @DarthSett!
Left some small comments, otherwise looks good.
| @@ -29,7 +29,7 @@ func dummyServer(t *testing.T, expectedToken, expectedID string, expectedVersion | |||
| } | |||
|
|
|||
| if have := r.Header.Get(xfer.ScopeProbeVersionHeader); expectedVersion != have { | |||
| t.Errorf("want %q, have %q", expectedID, have) | |||
| t.Errorf("want %q, have %q", expectedVersion, have) | |||
There was a problem hiding this comment.
Good catch, I wonder why the tests were passing before 🤔
There was a problem hiding this comment.
Could you please also add a similar check here for the user-agent field while you're at it?
There was a problem hiding this comment.
Oh yea. Sorry forgot to mention this change in the pr
There was a problem hiding this comment.
The tests are passing because this is just the bit that prints out what it found - the check is on the line before.
Co-Authored-By: Filip Barl <[email protected]>
|
I have committed the changes mentioned |
|
Thanks @DarthSett, Before we merge, could you please add a test for it? See #3720 (comment).
So far you've only fixed the existing test for |
|
@fbarl Added the check for user-agent and pushed it to the fork. Please review if it looks good or not |
Added the "user-agent" header with the value: "Scope/1.11.6" and added a test for it.
Fixes #3715.