preliminary rules/engine split #3742
Conversation
Signed-off-by: Cameron Wong <[email protected]>
cwong-ocaml
requested review from
aalekseyev,
ejgallego,
emillon and
fpottier
as code owners
Signed-off-by: Cameron Wong <[email protected]>
Signed-off-by: Cameron Wong <[email protected]>
Signed-off-by: Cameron Wong <[email protected]>
| let jbuild_plugin_ml = {jbp|let () = | ||
| let jbuild_plugin_ml = | ||
| {jbp| | ||
| let () = |
There was a problem hiding this comment.
You added an extra linebreak here. Why was this change needed?
There was a problem hiding this comment.
If it is not added, CI fails with a "code is not formatted properly" error.
There was a problem hiding this comment.
Hmm, but it wasn't there before...
There was a problem hiding this comment.
Yeah, I don't know what is or isn't going wrong. Basically all of the previous commits were failing CI with a formatting error, and running ocamlformat 0.14.3 on this file (alternatively, make fmt) also produces this change.
There was a problem hiding this comment.
Hmm, perhaps this file shouldn't be checked in at all if it is generated from the dune rules -- it looks like it is generated by wrapping some ppx around the contents of jbuild_plugin.ml and jbuild_plugin.mli and nothing else. I certainly don't think that it makes sense to check the formatter against such a file.
There was a problem hiding this comment.
this file was referenced in .ocamlformat-ignore, I think you just need to update this file.
There was a problem hiding this comment.
aha, that would do it, how careless of me.
I stand by what I said earlier, however -- I think that it does not make sense to check in such a file to git when it is entirely a wrapper around another. However, I suppose such a change can be argued over later.
Signed-off-by: Cameron Wong <[email protected]>
Signed-off-by: Cameron Wong <[email protected]>
The generated file is checked in to simplify the bootstrap procedure. It's not simple to generate things when bootstrapping because we don't have the full build system available. |
As per #3703 . This split was done mostly mechanically, taking build_system and all dependencies and putting them into
dune_engine, renamingdunetodune_rules. Then,open Dune_enginewas put at the top of all files indune_rules, and finally all the relevant build errors (due to other parts of the codebase referring toDune) were fixed.There should be no behavioral changes; this is only a refactoring.
I think, if there are no major objections, we should merge this soon, and we can work on bikeshedding the rest (which files should be in which library, etc) afterwards.
Signed-off-by: Cameron Wong [email protected]