You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It would be good to integrate ocamlformat into CI scripts to adapt it into more projects, and have a CI job which checks that the submitted PRs conform to the selected style. I'd propose to require the ocamlformat version explicitly OCAMLFORMAT="0.9" (avoiding issues like the latest for OCAML_VERSION which support we phased out over time -- it's utopic to assume all ci-script users will always want the same version of ocamlformat).
I've recently seen this commit which contains necessary changes (of course they need to be adapted to the scripts here).
if others think this is a good idea, and someone more familiar with the current code base could implement it, that'd be great! :)
I'm actually experimenting around this atm. I think a more satisfying way to deal with this would be to check formatting on a separate build.
That would allow us to get the results for this check faster since it doesn't require you to install any dependencies and to actually build or run the test for the project.
You can have a look at this PR I wrote. The ocamlformat build is separate from the other ones and uses images with pre-installed versions of ocamlformat and dune. Right now they're hosted on my dockerhub but it would be nice to have them on the ocaml one.
It would be good to integrate ocamlformat into CI scripts to adapt it into more projects, and have a CI job which checks that the submitted PRs conform to the selected style. I'd propose to require the ocamlformat version explicitly
OCAMLFORMAT="0.9"
(avoiding issues like thelatest
forOCAML_VERSION
which support we phased out over time -- it's utopic to assume all ci-script users will always want the same version of ocamlformat).I've recently seen this commit which contains necessary changes (of course they need to be adapted to the scripts here).
if others think this is a good idea, and someone more familiar with the current code base could implement it, that'd be great! :)
//cc @NathanReb @emillon @avsm @samoht (further discussion also in mirage/ocaml-cstruct#234)
The text was updated successfully, but these errors were encountered: