Skip to content
This repository has been archived by the owner on Nov 24, 2022. It is now read-only.

add a HTTP header to hint the server only the car header is needed #21

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

merlinran
Copy link
Contributor

No description provided.

@@ -134,11 +134,14 @@ func (u *HTTPURI) String() string {
return u.uri
}

func (u *HTTPURI) getRequest(ctx context.Context) (*http.Response, error) {
func (u *HTTPURI) getRequest(ctx context.Context, extraHeaders ...headerModifier) (*http.Response, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bit overkill - just want to allow callers passing header optionally.

Copy link
Contributor

@jsign jsign Jul 27, 2021

Choose a reason for hiding this comment

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

SGTM. This allows not only adding but mostly doing anything; eventually we can pas a map and simply add things to avoid one things to mess up with another (and possible the order in which things are applied; responsibility of the caller anyway).

This is a private method anyway, so not a big deal. SGTM.

@merlinran merlinran requested a review from jsign July 26, 2021 19:44
Signed-off-by: Merlin Ran <merlinran@gmail.com>
@@ -134,11 +134,14 @@ func (u *HTTPURI) String() string {
return u.uri
}

func (u *HTTPURI) getRequest(ctx context.Context) (*http.Response, error) {
func (u *HTTPURI) getRequest(ctx context.Context, extraHeaders ...headerModifier) (*http.Response, error) {
Copy link
Contributor

@jsign jsign Jul 27, 2021

Choose a reason for hiding this comment

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

SGTM. This allows not only adding but mostly doing anything; eventually we can pas a map and simply add things to avoid one things to mess up with another (and possible the order in which things are applied; responsibility of the caller anyway).

This is a private method anyway, so not a big deal. SGTM.

@merlinran merlinran merged commit bc470e1 into main Jul 27, 2021
@merlinran merlinran deleted the merlin/car-header-only branch July 27, 2021 00:02
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.

2 participants