cohttp: move new client and server modules into a generic module #1003
Conversation
to avoid shadowing Client and Server modules when opening Cohttp. This preserves backward compatibility for a very small price and improves library ergonomics since right now many project tend to open Cohttp to have the Headers and Response modules directly accessible. Signed-off-by: Marcello Seri <[email protected]>
Signed-off-by: Marcello Seri <[email protected]>
mseri
force-pushed
the
cohttp6-usability
branch
from
2007025
to
59af795
Compare
|
Ping @samoht @mefyl @talex5. I have seen this failure in the opam-repository revdeps: |
|
We don't test the readme directly afaik, so it may as well be broken |
|
tldr: IMO fine in 6.0.0 AFAIK there is no consensus in the Ocaml community regarding backward compatibility and versioning and in my opinion we really need one. Some developers / company have their own approach, but most of the time I don't feel like there is any guarantee as to what package versions to pin to avoid breakage, and I tend to err on the paranoid side. My personal take on this that the absolute worse thing is compilation and CIs breaking one morning because a new version of an external dependency was released. More so since this often does not happen immediately because build images are often cached or use lockfile, and will happen randomly when the image or lockfile is regenated. It follows that we need a clear guarantee of no API breakage depending on the versionning, and if we're going with semver, which is not perfect but is the most universally adopted thing, that means no API breakage except for major updates, so my opinion on that is that this is fine for cohttp 5 only. I think it's reasonable to expect some possible breakage if you jump major versions, and if you don't want that you can (should) On another note there are ways to have API change on minor versions while providing backward compatibility modules, having the better of both worlds provided some additional work for the maintainer. We do this internally quite successfully, and I've seen other do. I don't know if anyone ever wrote down a ruleset, but I'd be happy to submit how we proceed for comments. Edit: fixed versions |
|
@mefyl I see your point but I am not sure how it applies here. We don't give any guarantee across major versions in cohttp either, and in fact most projects in the opam-repository are already incompatible with 6.0.0 due to the many internal changes. When I reviewed the original PR it made sense to have client and server directly as toplevel modules in cohttp, but when testing it became inconvenient because it plays badly with We are releasing alphas and betas to check the effects of the API changes on the ecosystem and discuss in anything in the api needs improvement, so I think the revdeps and going through the failures there are useful. In this case I think it is worth making this change because it is more annoying to have to figure out the shadowed module failure all over the place or having to call server with some other name to avoid it. |
|
I botched the versioning in my message, I meant fine in 6.0.0. We agree then, and alpha/betas are of course fine, the name imply you will encounter rough edges. |
Absolutely! I suggested this change since I think it is a usability improvement, I didn't mean to suggest that we had to keep backward compatibility at all costs |
to avoid shadowing Client and Server modules when opening Cohttp.
This preserves backward compatibility for a very small price and improves library ergonomics since right now many project tend to open Cohttp to have the Headers and Response modules directly accessible.