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

Cleanup HttpClient #639

Merged
merged 15 commits into from
Jul 21, 2019
Merged

Cleanup HttpClient #639

merged 15 commits into from
Jul 21, 2019

Conversation

iNoles
Copy link
Collaborator

@iNoles iNoles commented May 23, 2019

Description

HttpClient deserve to be refracting with many areas. As for Hook, InputStream is now nullable. I am moving httpExchangeFailed to the different area. I added the connect() to make sure it is connected. I am planning to add more later on.

Type of change

Check all that apply

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (a change which changes the current internal or external interface)
  • This change requires a documentation update

How Has This Been Tested?

In case you did not include tests describe why you and how you have verified the
changes, with instructions so we can reproduce. If you have added comprehensive
tests for your changes, you may omit this section.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation, if necessary
  • My changes generate no new compiler warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Inspect the bytecode viewer, including reasoning why

iNoles added 2 commits May 6, 2019 01:36
interpretResponseStream should have InputStream as nullable.
Move hook.httpExchangeFailed to different area.
@iNoles iNoles self-assigned this May 23, 2019
@codecov
Copy link

codecov bot commented May 23, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@5cc452c). Click here to learn what that means.
The diff coverage is 54.16%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #639   +/-   ##
=========================================
  Coverage          ?   71.39%           
  Complexity        ?      297           
=========================================
  Files             ?       57           
  Lines             ?     1489           
  Branches          ?      192           
=========================================
  Hits              ?     1063           
  Misses            ?      334           
  Partials          ?       92
Impacted Files Coverage Δ Complexity Δ
...ava/com/github/kittinunf/fuel/stetho/StethoHook.kt 45.45% <ø> (ø) 0 <0> (?)
...in/kotlin/com/github/kittinunf/fuel/core/Client.kt 0% <ø> (ø) 0 <0> (?)
...tlin/com/github/kittinunf/fuel/core/DefaultHook.kt 100% <100%> (ø) 2 <1> (?)
...in/com/github/kittinunf/fuel/toolbox/HttpClient.kt 76.85% <52.17%> (ø) 29 <3> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cc452c...a71a353. Read the comment docs.

@iNoles iNoles marked this pull request as ready for review May 24, 2019 02:15
@iNoles iNoles requested review from markGilchrist, kittinunf and SleeplessByte and removed request for markGilchrist and kittinunf May 24, 2019 02:16
@kittinunf
Copy link
Owner

This looks like it is good to go, right? 😄

@iNoles iNoles removed the request for review from markGilchrist July 6, 2019 03:53
@iNoles iNoles requested a review from SleeplessByte July 6, 2019 04:07
@iNoles
Copy link
Collaborator Author

iNoles commented Jul 21, 2019

@SleeplessByte it is ready to merge with your suggestions.

Sent with GitHawk

@SleeplessByte SleeplessByte merged commit f46bd4f into master Jul 21, 2019
@delete-merged-branch delete-merged-branch bot deleted the cleanup_httpclient branch July 21, 2019 22:01
@kittinunf kittinunf mentioned this pull request Jul 22, 2019
8 tasks
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.

3 participants