mirage / ocaml-github

GitHub APIv3 OCaml bindings
ISC License
100 stars 61 forks source link

Tidy up compile errors. #246

Closed tmcgilchrist closed 3 years ago

tmcgilchrist commented 3 years ago

Running opam exec -- dune build @install @check @runtest shows up errors in the tests. Based off the warnings from ocaml-ci

The only questionable piece here is deleting the test_javascript.ml file which didn't seem to work anyway.

tmcgilchrist commented 3 years ago

A few warnings on OCaml 4.12, the errors in github_core.mli seem spurious, not sure about unerasable-optional-argument which is fine on previous OCaml versions.

File "lib/github_core.mli", line 24, characters 12-15:
24 | module Make(Env: Github_s.Env)(Time: Github_s.Time)(CL : Cohttp_lwt.S.Client)
                 ^^^
Warning 67 [unused-functor-parameter]: unused functor parameter Env.
File "lib/github_core.mli", line 24, characters 31-35:
24 | module Make(Env: Github_s.Env)(Time: Github_s.Time)(CL : Cohttp_lwt.S.Client)
                                    ^^^^
Warning 67 [unused-functor-parameter]: unused functor parameter Time.
File "lib/github_core.mli", line 24, characters 52-54:
24 | module Make(Env: Github_s.Env)(Time: Github_s.Time)(CL : Cohttp_lwt.S.Client)
                                                         ^^
Warning 67 [unused-functor-parameter]: unused functor parameter CL.
File "lib/github_core.ml", line 239, characters 19-21:
239 |     let repo_refs ?ty ~user ~repo =
                         ^^
Error (warning 16 [unerasable-optional-argument]): this optional argument cannot be erased.
avsm commented 3 years ago

All looks like a strict improvement, merging! It would be good to fix the 4.12 errors too; I think we need a breaking interface change in repo_refs to add a () at the end, as it is indeed true that ?ty cannot be erased.