ocaml-cohttp
ocaml-cohttp copied to clipboard
API inconsistencies between lwt and async
Here's a single example from Server:
val respond_string :
?headers:Cohttp.Header.t ->
status:Cohttp.Code.status_code ->
body:string -> unit -> (Response.t * Cohttp_lwt_body.t) Lwt.t
and
val respond_with_string :
?flush:bool ->
?headers:Cohttp.Header.t -> ?code:Cohttp.Code.status_code ->
string -> response Deferred.t
The two functions are (almost) functionally identical, but they have slightly different names and slightly different parameters. I wonder if this was intentional? i.e. to make the user read the code carefully after the switch to another library.
Some differences are indeed intentional, but others are just inconsistencies that you've correctly pointed out.
Would you accept a PR renaming/deprecating some of these?
Sure. Just please don't make breaking changes.
A few points:
- Use the
[@@deprecated ..]to deprecate functions - Prefer names without "filler" words such
with - Reordering labelled/optional arguments doesn't count as breakage. So feel free to just make those consistent on the spot.
@rgrinberg @superbobry I just created a PR for this issue. I was careful not to introduce any breaking change but did add an optional argument flush to the Lwt.Server.respond_string function. If the changes are approved, I could work on renaming the other functions and their optional/labeled arguments.