Update HTTP.jl compat #19
Conversation
Codecov Report
@@ Coverage Diff @@
## main #19 +/- ##
==========================================
- Coverage 67.27% 67.27% -0.01%
==========================================
Files 17 17
Lines 602 605 +3
==========================================
+ Hits 405 407 +2
- Misses 197 198 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Thanks for this!
I think go all in v1. Much simpler code for us to maintain and hopefully people are adopting the HTTP V1 across Julia. I think this will simplify this PR too :) |
Co-authored-by: Mal Miller <59854849+mmiller-max@users.noreply.github.com>
Yes makes sense !
Sure, I started quite defensively to maintain compat but it's better to simplify since it's going to be the new HTTP only ! |
|
Simplified
to
|
|
@mmiller-max is there anything extra you want in this PR? Maybe I can help with that too! |
|
Thanks @pankgeorg and @mmiller-max! It would be great to have this released soon. |
| while !eof(ws) | ||
| data = readavailable(ws) | ||
| for data in ws | ||
| nothing; |
There was a problem hiding this comment.
Does the semi colon add anything here?
There was a problem hiding this comment.
not really, it just looked nicer at the time 😅
Co-authored-by: Mal Miller <59854849+mmiller-max@users.noreply.github.com>
|
Looks good to me. Just one question left to check and I'd ideally test it with a server that already had working subs. Please can you bump the version so once it's in we can get it released? |
|
Thanks again @pankgeorg ! |
|
Bump. Would be great to get this merged soon since we have a bunch of packages that are held back on the HTTP@v1 upgrade by GraphQLClient. Thanks! |
|
Sorry for the delay here. Merged and registered! |
Queries seem to work but I haven't tested subscriptions at allWhat do you think we should do here? Write some patch code to maintain compat or go all in HTTP 1.0?
Cheers, P!