ocaml / ocaml/dune

adding new formatters can break older projects

Open
#3,642 11 comments 0 reactions 1 assignee View on GitHub

@emillon is already working on this.

Since Jul 21, 2020.

config enhancement
Dominant language
OCaml
Stars
1.9k
Forks
500
Avg merge
15h 21m
Merged PRs (30d)
277

Description

In the ocaml-ci, we have a "lint fmt" step which probes the project metadata to determine the formatters to run over the project. This is primarily for dune files and ml[i] files via ocamlformat.

The problem arises when we try a dune build @fmt with ocamlformat installed but without a .ocamlformat file, at which point dune tries to invoke ocamlformat even if there is no way it can work:

File x.ml          
Warning: Ocamlformat disabled because [--enable-outside-detected-project] is not set and no [.ocamlformat] was found within the project (root: ../../../x)

I think there's an erroneous default in dune that makes this difficult to work around:

By default, formatting will be enabled for all languages and dialects present in the project that dune knows about.

This default means that projects have to specifically add (formatting disabled) to turn it off for OCaml code, but to enable it for dune files.

Wouldn't it be better to specifically version formatters used by a project, so that a future dune client understanding a new formatter wouldn't cause older format lint tests to fail? Specifically, this means flagging that ocamlformat is enabled specifically for a project. We could minimise breakage by taking the presence of an .ocamlformat file to mean (formatting enabled), and the absence to be (formatting disabled).

See https://github.com/ocurrent/ocaml-ci/issues/224

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.