Skip to content
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

Enforce formatting of the codebase? #160

Open
Niols opened this issue Apr 28, 2023 · 0 comments
Open

Enforce formatting of the codebase? #160

Niols opened this issue Apr 28, 2023 · 0 comments

Comments

@Niols
Copy link
Member

Niols commented Apr 28, 2023

PR #158 enforces systematic checks that the code base is correctly indented with ocp-indent. This was already the case pretty much everywhere. This does raise the question, though: should we enforce something stronger? As far as I can see, we have four possibilities:

  1. do not enforce anything
  2. enforce ocp-indent
  3. enforce OCamlFormat
  4. enfore Topiary

Here are my personal opinions on the topic:

  1. I am fine not enforcing anything. We have done that for a while and it has worked fine. However, I am not sure it is enough: has worked so far because we used similar IDEs.

  2. This is what Formatting of OCaml and Dune #158 introduces. I think it costs very little because most of the code is already compatible with ocp-indent. I would be up for doing at least this. This will prevent future discussions on whether a file is indented correctly.

  3. I am not a big fan of OCamlFormat; I've never really managed to use it in a satisfactory way. Also, it provides many options which could easily lead to endless debates on how to configure it exactly, which I think is counterproductive. However, it is a very easy option to implement, it is available via OPAM and a fair amount of package managers. It would be easy to enforce in CI as well.

  4. I have had much more success with Topiary in term of formatting; it being opinionated also helps prevent some friction between developers. However, it is hard to get at the moment and will probably be very unstable from one version to the other, so I don't think it is mature enough for us at this point.

My vote would therefore be to go for option 2. WDYT @yurug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant