Remove devkit dependency#11
Conversation
| Devkit.Files.save_as (Path.project target_file) ~mode:0o600 (fun oc -> Out_channel.output_string oc contents) | ||
| let target_file = Path.project target_file in | ||
| let flags = [ Open_wronly; Open_creat; Open_trunc; Open_binary ] in | ||
| Out_channel.(with_open_gen flags 0o600 target_file (fun oc -> output_string oc contents)) |
There was a problem hiding this comment.
this is not nice, see comment in save_as why
| Devkit.Control.with_open_out_temp_file ~temp_dir ~mode:[ Open_wronly; Open_creat; Open_excl ] f | ||
| let tmpfile, tmpfile_oc = | ||
| Filename.open_temp_file ~mode:[ Open_wronly; Open_creat; Open_excl ] ~temp_dir "" ".passage" | ||
| in | ||
| Fun.protect ~finally:(fun () -> try Sys.remove tmpfile with _ -> ()) (fun () -> f (tmpfile, tmpfile_oc)) |
| let inject_list = List.map inject | ||
| let project_list = List.map project |
There was a problem hiding this comment.
proper replacement would be = id with correct signature
| Out_channel.with_open_text (show_path recipients_file_path) (fun oc -> | ||
| List.iter (fun line -> Printf.fprintf oc "%s\n" line) recipients_names_with_root_group) |
| let to_string exn = | ||
| match exn with | ||
| | Unix.Unix_error (e, f, s) -> sprintf "Unix_error %s(%s) %s" f s (Unix.error_message e) | ||
| | exn -> to_string exn |
There was a problem hiding this comment.
this is strange way to "register" exn printer
There was a problem hiding this comment.
Hmm, I'm not sure what to do here. The code was introduced here 9af2e7a#diff-17dd93d35399c2557c20172cf0d2e6dd89cdd2ac8fb085fd2fcf68f1614a54c5 by @thatportugueseguy IIUC.
I'm guessing that what you're alluding to is that we could:
let () =
Printexc.register_printer (fun exn ->
match exn with
| Unix.Unix_error (e, f, s) -> Some (sprintf "Unix_error %s(%s) %s" f s (Unix.error_message e))
| _ -> None)
at the top this module? This would give the same effect as @thatportugueseguy's code (in die) but would also affect all calls to Printexc.to_string.
There was a problem hiding this comment.
This is a part of the Devkit exn printer's code. I removed the parts that weren't relevant for passage, and incorporated the rest in the die function, as we didn't have a use for it anywhere else.
This isn't registering the printer because it's not exposed. Do you think it should be more inlined, then?
There was a problem hiding this comment.
yes, use register_printer, affecting all the code is good obv
|
|
||
| let inject x = x | ||
| let project x = x | ||
| let inject_list = List.map inject |
|
|
||
| module Secret_name = struct | ||
| include Devkit.Fresh (String) () | ||
| type t = string |
There was a problem hiding this comment.
this defeats the purpose of having Secret_name.t, previously it was abstract, now it is string alias
With this change, passage is a easier to install from opam because the dependency cone is greatly reduced. In particular:
Most things passage used from devkit can be found in the OCaml stdlib these days.
Builds & passes tests, code is mainly AI written but I've reviewed it. Seems to be working fine but review & suggestions for additional testing is welcome!
Finally, I think the dependency cone can be reduced even further by removing bos, extunix, replacing re2 with re, and making ppx_expect
--with-test. Happy to work on it or to provide advice :)