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.

Sign up for GitHub

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

Jump to bottom

Copy HTTP client #261

Merged
merged 3 commits into from
Oct 2, 2020
Merged

Copy HTTP client #261

merged 3 commits into from
Oct 2, 2020

Conversation

swithek
Copy link
Contributor

@swithek swithek commented Sep 22, 2020

Closes #260

@swithek swithek requested a review from nhooyr as a code owner September 22, 2020 10:57
@swithek swithek changed the title Copy http client Copy HTTP client Sep 22, 2020
nhooyr
nhooyr reviewed Sep 24, 2020
View reviewed changes
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

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

Thanks for contributing @swithek!

Thoughts on instead of removing the timeout completely, copying the client and removing it in Dial and then using whatever timeout is on the client on the Dial context?

That way things would just work as expected.

@swithek
Copy link
Contributor Author

swithek commented Sep 24, 2020

@nhooyr that's a good point, I will submit an update soon.

@swithek swithek requested a review from nhooyr September 24, 2020 07:38
nhooyr
nhooyr suggested changes Sep 24, 2020
View reviewed changes
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

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

Looks great otherwise!

@swithek swithek requested a review from nhooyr September 27, 2020 08:39
@swithek
Copy link
Contributor Author

swithek commented Sep 30, 2020

@nhooyr is there anything else that needs improving?

@nhooyr
Copy link
Contributor

nhooyr commented Oct 2, 2020

Nope, looks great.

Thanks for contributing @swithek <3

@nhooyr nhooyr merged commit b00726c into coder:master Oct 2, 2020
@dpastoor dpastoor mentioned this pull request Oct 9, 2020
Closed
nhooyr added a commit that referenced this pull request Jan 9, 2021
@nhooyr
Merge pull request #261 from swithek/copy-http-client
e19ed0c 
Allow *http.Client with Timeout set
nhooyr added a commit that referenced this pull request Jan 9, 2021
@nhooyr
fixup! Merge pull request #261 from swithek/copy-http-client
84466c0
nhooyr added a commit that referenced this pull request Jan 9, 2021
@nhooyr
fixup! Merge pull request #261 from swithek/copy-http-client
ebf292d
nhooyr added a commit that referenced this pull request Jan 9, 2021
@nhooyr
Merge branch 'master' into merge-master
75f9acc 
- #261
- #266
- #273
nhooyr added a commit that referenced this pull request Jan 9, 2021
@nhooyr
Merge branch 'master' into merge-master
0a50811 
There were a few PRs merged into the master branch that were then not
merged into the dev branch. This branch merges those changes in cleanly.

- #261
- #266
- #273
@nhooyr nhooyr mentioned this pull request Jan 9, 2021
Merged
nhooyr added a commit that referenced this pull request Jan 9, 2021
@nhooyr
Merge branch 'master' into merge-master
06beb14 
There were a few PRs merged into the master branch that were then not
merged into the dev branch. This branch merges those changes in cleanly.

- #261
- #266
- #273
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhooyr nhooyr nhooyr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error should not be returned on non-zero http client timeout
2 participants
@swithek @nhooyr