[WIP] allow use of ocamlformat 0.20#265
Conversation
|
If I remember correctly, we didn’t up ocamlformat version because the formatting was different, so people would have had the whole code base changed each time they commit if we use different version. I suppose the option you add enforces new ocamlformat to use the old way. But at the end, we know all contributors and we have control on the dependencies, so why not ditch retro compatibility and coordinate to use the new format everywhere? |
That's right.
We could. I just went for the most hassle-free way. Is updating the format something we want? |
|
Unless there is a specific reason to use the 0.19 version, I'd be in favor of upgrading to the latest version given we can keep the default behavior of 0.19. |
|
Historically, locking the version was a way to ensure every contributor would not blast formatting diffs through every changes, because we didn't bother to find a way to effectively lock the format conf. So unless we have a futur-proof way to enforce that, upgrading would still need a version lock and futur upgrade to go through the same process. Here the point is to avoid compatibility issues, so this warrants the pain of changing the whole codebase (which can be reduced to naught with a single formatting only commit and the right rebase script (been there, done that)). But even then it seems it's not an issue here. But I would advise locking nonetheless to avoid a switch on v0.18 or v0.21 to break everything |
|
The simpler would be to upgrade to a version that at least allow to use any version of ocaml 4.xx. The 0.22.4 seems to be the smallest one with that property : https://opam.ocaml.org/packages/ocamlformat/ocamlformat.0.22.4/ |
d563973 to
40cc835
Compare
|
I've updated the MR to use ocamlformat.0.22.4. |
|
I see no reason, once we upgrade ocamlformat, to not upgrade as far as we can. Is there an argument against ocamlformat 0.27? The minimum compiler version is the same as 0.22.4. |
|
As far as I can tell, it’s good enough as is. |
I agree with this, the idea of going to the minimal version for 4.14.X was to limit the changes of the default behavior of ocamlformat but we could definitely go to 0.27. I think we can merge as is and, if we need it, upgrade later on. |
3201645 to
61e3e32
Compare
|
I've updated it to the latest ocamlformat version, |
This is the ocamlformat version that works with every OCaml 4 version. This is needed as there's a bug when using the latest libc with ocaml 4.13.
d5cb583 to
9836ab4
Compare
|
Actually, ocamlformat.0.25.1 fixes some stuff, which would reformat some of our code: Nested non-indented ifs such as the one found in (if flags.Dgfip_options.flg_debug then
if flags.nb_debug_c <= 0 then
let nb = match nb_debug with [ nb ] -> nb | _ -> assert false in
Format.fprintf fmt "#define NB_DEBUG %d\n" nb
else
let i =
List.fold_left
(fun i nb ->
Format.fprintf fmt "#define NB_DEBUG%02d %d\n" i nb;
i + 1)
1 nb_debug
in
assert (i = flags.nb_debug_c + 1));
[...]As such, I've bumped ocamlformat to the highest version which causes no breaking changes, 0.24.1. |
Rationale
On newest version of the libc, the ocaml compiler version 4.13 is broken. A fix
is present in ocaml 4.14.2.
As of now, the only thing preventing the installation of 4.14.2 is the dependency
on ocamlformat 0.19.0. This PR proposes to allow ocamlformat version 0.20, which
would allow the usage of ocaml 4.14.2. Of course, we aim to be fully retrocompatible,
which means that current users of ocamlformat 0.19.0 and ocaml 4.13 should notice
no difference.