-
Notifications
You must be signed in to change notification settings - Fork 98
Use (format-dune-file)
instead of dune format-dune-file
#1338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Thanks @nojb! One question I have regarding this: shouldn't it add a dependency on |
Yes, indeed. I pushed a fix (I think ; I am not very familiar with OPAM). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nojb! That looks good.
The benchmark CI failure seems to be related to ocaml/dune#11045.
It does not happen on ocaml-ci, which install deps and then call dune. On the contrary, the benchmark ci seems to pin everything and install them with opam, which calls dune subst
and that is what is breaking.
Maybe that would be better to wait for the ocaml/dune#11045 fix to be released before raising dune lang
if that is what triggers the bug?
The new fix was merged. It should be available in 3.20. |
4661efe
to
913e49e
Compare
With the newly released
|
I'm not sure it contains the patch from: ocaml/dune@ad06a13. |
I think it does include it. The old error message was:
and it is now:
I guess the error is either on the benchmark CI or on dune's side, so it would be a pity to block this PR on it. I don't know how much the benchmark CI is used, at least I struggle with its UI... |
Hello,
You are currently calling
dune format-dune-file
from one of your tests. This command does not honour the Dune lang version used in the project which means that your test will break whenever we change the formatting style in Dune.In Dune 3.18 we introduced a built-in action
(format-dune-file <src> <dst>)
precisely for this purpose. This action honours the current Dune lang version and does not require shelling out to an external process.Note that
dune format-dune-file
may evolve in ways that may not be backwards compatible in the near future (as the CLI interface does not have any backwards compatibility or versioning guarantees), so we are encouraging users of this command to switch to the action instead.See ocaml/dune#10892 (comment) for some of the context.