Skip to content

Update "Accept" header for github.com requests#3803

Merged
mislav merged 3 commits intotrunkfrom
http-accept-header
Jun 11, 2021
Merged

Update "Accept" header for github.com requests#3803
mislav merged 3 commits intotrunkfrom
http-accept-header

Conversation

@mislav
Copy link
Contributor

@mislav mislav commented Jun 8, 2021

The antiope-preview has graduated in github.com and no longer needs activating. However, we still need it for GHES requests.

Bonus: this adds tests for our base HTTP client that handles all requests and adds headers like User-Agent, Authorization, and Accept.

mislav added 2 commits June 8, 2021 19:21
The `antiope-preview` has graduated in github.com and no longer needs
activating. However, we still need it for GHES requests.
Copy link
Contributor

@samcoe samcoe left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for adding these tests! I left one small non-blocking comment.

},
host: "github.com",
wantHeader: map[string]string{
"authorization": "",
Copy link
Contributor

Choose a reason for hiding this comment

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

I found this test just a tad confusing in that it made it seem like the Accept argument also controlled if the authorization header was set when it is just not set due to the config not having authorization information for github.com. It might be a bit less confusing if we added two tests specifically for the authorization header (one where there is config auth token value for host and one without).

@mislav mislav enabled auto-merge June 11, 2021 12:33
@mislav mislav merged commit af90f72 into trunk Jun 11, 2021
@mislav mislav deleted the http-accept-header branch June 11, 2021 14:10
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