search

ocaml-ppx / ppx_import

Less redundancy in type declarations and signatures
MIT License
89 stars 28 forks source link

add support for OCaml 4.13 #63

Closed tatchi closed 2 years ago

tatchi commented 2 years ago

First, commit adds OCaml 4.13 to the CI config to demonstrate that it currently fails. Second commit adds support for OCaml 4.13

ejgallego commented 2 years ago

Thanks @tatchi , will review and merge ASAP.

By the way, do any of you folks remember how to disable the ocaml-ci check that is making our CI red?

tatchi commented 2 years ago

By the way, do any of you folks remember how to disable the ocaml-ci check that is making our CI red?

Never used that myself but isn't that something you can configure in the repo's settings (in the Integrations menu) ?

ejgallego commented 2 years ago

Never used that myself but isn't that something you can configure in the repo's settings (in the Integrations menu) ?

@tatchi thanks, indeed there it is, but I lack the permissions to update it, maybe @kit-ty-kate knows what we should do with it [or point us to the right person]

gasche commented 2 years ago

(@ejgallego You have "Admin" permissions to the ppx_import repository, so I don't know what's going on. Are those integrations an organization-level setting?)

ejgallego commented 2 years ago

(@ejgallego You have "Admin" permissions to the ppx_import repository, so I don't know what's going on. Are those integrations an organization-level setting?)

Seems so, it was discussed but I don't remember the details sorry :/ (and thanks for the help)

kit-ty-kate commented 2 years ago

ocaml-ci does not have any configuration (it’s in its "opinionated philosophy" or something like that i believe). I had a look locally and your issue seems to be that dune build @check does not work for some reason.

Just a wild guess but I think dune is scanning every directories in search of .ml files and compiling them to create .cmti. However not all the .ml files inside src/compat are used all the time based on the compiler version if i understand correctly. So I believe you might need to be specific in https://github.com/ocaml-ppx/ppx_import/blob/01f937cbbd3bcd5b0e540951b115244e14a79366/src/dune#L10 to only depend on the required files.

If you really want to disable it all together I believe the url is here: https://github.com/apps/ocaml-ci/installations/new

ejgallego commented 2 years ago

Oh, good catch @kit-ty-kate , thanks a lot !