Revert conduit 3 changes #741
Conversation
|
I'm fine with this. We'll revisit the conduit changes (which were very much heading in the right direction) once we have the maintainer bandwidth to figure out the revdeps cost to the community. Since cohttp 3.x has been tagged already, we cannot re-release, so lets bump to cohttp 4 for future API changes, and conduit2 can come back in cohttp 5. @samoht we need to make the CI pass. I can take a quick look at that in a few hours and push to your branch. @mseri -- this ok with you? |
|
I'll fix the CI. I also haven't realised that cohttp 3 was released (but it has |
|
That's right -- it got marked as available:false fairly soon after because of the revdeps changes, but it's there. Unfortunately the new package names will show up as a result on searches. We'll need to edit the opam metadata. |
|
I think it will be hard to cherrypick all the fixes that went in during the preparation for 3.0.0 and would be a pity to lose them. Why don't we just wait and update the code once the new conduit is released? Do we really need to sync the version numbers of conduit and cohttp? 3.0.0 is tagged but not released... we could always tag 3.0.1 and release that as the official 3.0.0 once appropriate. (In fact, since it is unavailable on opam-repository, we could really remove it there -- still preserving the 3.0.0 tag and release here unofficially). Or release 3.0.0~alpha and leave it there for people that want to try the current API, with a huge warning that there are further upcoming breaking changes. |
Nobody has time right now to focus on the new conduit, so let's not block ourselves on this and let's continue to improve (and release) cohttp. For instance @lyrm is interested to contribute to improve cohttp headers, so my motivation is to unblock her asap :-) Also while doing this I noticed a few clean-up that I'd like to do next (use ocamlformat, move src and tests around). |
|
(also I'll double check but I think I've cherry-picked everything that needed to be cherry-picked :p) |
|
In that case, go ahead! |
|
I think we can release this as 3.0.1 no matter what, keeping 3.0.0 unavailable or completely removing it from opam. |
|
I think we will need to fix the server examples in the README, we should probably use mdx to compile them and keep them updated (I was doing it manually by also having them in the examples folder) |
Ok I actually missed a few renaming commits. Fixing that now. |
|
I'm fine with this being 3.x, but make it 3.1.0 at least. It really isn't a point release over what 3.0.0 was tagged as ;-) |
|
I have no preference really, just wanted to avoid skipping 3 since it is effectively unreleased 😜 |
|
Skipping the atomic number of lithium does seem like a pity! Let's go with 3.1.0 @samoht |
samoht
force-pushed
the
revert-conduit3
branch
from
a33de38
to
9fa8152
Compare
|
Ok I think I've cherry-picked everything that I could do now. Please check that the diff looks ok. The one that makes me very sad is reverting support for client-side TLS certificates. I am so sad that I might make a small patch to conduit to restore this... |
samoht
force-pushed
the
revert-conduit3
branch
2 times, most recently
from
32173ef
to
0e7ef77
Compare
|
Ok all fixed now -- I think it's ready to be merged once everyone is happy with the revert. |
samoht
force-pushed
the
revert-conduit3
branch
from
c7bacb0
to
6592133
Compare
|
The client-side CA cert patch has been backported to conduit 2: mirage/ocaml-conduit#374 |
|
Fantastic! I went over the changes again, it looks fine to me. @avsm do you also give the green light for a merge? |
…ohttp-top, cohttp-async and cohttp-mirage (4.0.0) CHANGES: - cohttp.response: fix malformed status header for custom status codes (@mseri @aalekseyev mirage/ocaml-cohttp#752) - Remove dependency to base (@samoht mirage/ocaml-cohttp#745) - fix opam files and dependencies - add GitHub Actions workflow (@smorimoto mirage/ocaml-cohttp#739) - lwt_jsoo: Forward exceptions to caller when response is null (@mefyl mirage/ocaml-cohttp#738) - Remove wrapped false (@rgrinberg mirage/ocaml-cohttp#734) - Use implicit executable dependency for generate.exe (@TheLortex mirage/ocaml-cohttp#735) - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686) - [reverted] breaking changes to client and server API to use conduit 3.0.0 (@dinosaure mirage/ocaml-cohttp#692). However, as the design discussion did not reach consensus, these changes were reverted to preserve better compatibility with existing cohttp users. (mirage/ocaml-cohttp#741, @samoht)
…ohttp-top, cohttp-async and cohttp-mirage (4.0.0) CHANGES: - cohttp.response: fix malformed status header for custom status codes (@mseri @aalekseyev mirage/ocaml-cohttp#752) - Remove dependency to base (@samoht mirage/ocaml-cohttp#745) - fix opam files and dependencies - add GitHub Actions workflow (@smorimoto mirage/ocaml-cohttp#739) - lwt_jsoo: Forward exceptions to caller when response is null (@mefyl mirage/ocaml-cohttp#738) - Remove wrapped false (@rgrinberg mirage/ocaml-cohttp#734) - Use implicit executable dependency for generate.exe (@TheLortex mirage/ocaml-cohttp#735) - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686) - [reverted] breaking changes to client and server API to use conduit 3.0.0 (@dinosaure mirage/ocaml-cohttp#692). However, as the design discussion did not reach consensus, these changes were reverted to preserve better compatibility with existing cohttp users. (mirage/ocaml-cohttp#741, @samoht)
As we are delaying the release of conduit 3.0 again (because we are still not fully happy with the complexity added by the new API) I am temporary reverting #692. As a few changes have been made to master since then, I had to do it manually. Hopefully I haven't broken anything.
/cc @dinosaure and @lyrm