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.

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

Remove unused WithTimeout method #11420

Merged
merged 9 commits into from
Sep 13, 2022
Merged

Remove unused WithTimeout method #11420

merged 9 commits into from
Sep 13, 2022

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Sep 8, 2022

What does this PR do? Why is it needed?
Remove unused WithTimeout wrapper for HTTP client, I saw this was unused while working on something else and thought to remove it for code health

Which issues(s) does this PR fix?
N/A

Other notes for review
I'm not sure what the original intention was, maybe we do want to use it? cc @kasey

@terencechain terencechain requested a review from a team as a code owner September 8, 2022 13:40
@terencechain terencechain self-assigned this Sep 8, 2022
@terencechain terencechain added Ready For Review A pull request ready for code review OK to merge labels Sep 8, 2022
@prestonvanloon
Copy link
Member

Why is this unused?

@terencechain
Copy link
Member Author

Why is this unused?

I'm not sure what the original intent was. cc @kasey

@kasey
Copy link
Contributor

kasey commented Sep 13, 2022

Why is this unused?

I added it because I was wrapping the underlying http client and thought it would be important to give people access to the underlying timeout setting via an option. I don't remember if I used it at some point or not. I'm fine with removing it and presumably at some point in the lifetime of the library someone else will add it :)

@terencechain terencechain merged commit 3b8a3c2 into develop Sep 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the unused branch September 13, 2022 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants