search

mirage / irmin

Irmin is a distributed database that follows the same design principles as Git
https://irmin.org
ISC License
1.83k stars 154 forks source link

Upgrade to ocamlformat 0.26.0 #2262

Closed Julow closed 1 year ago

Julow commented 1 year ago

The formatting of module arguments changed and this cause large diffs on Irmin.

The aim of this commit is to gather feedback.

Changelog can be found here: https://github.com/ocaml-ppx/ocamlformat/blob/main/CHANGES.md

art-w commented 1 year ago

Thanks, the changes looks a lot better overall :)

Julow commented 1 year ago

Why the two additional spaces? It seems like the general rhythm is 2 spaces per indentation. Why should the arguments be indented twice under module (versus the original 1 for the include line).

This is the existing formatting for module arguments so changing it would cause huge diffs. A bigger indentation is used to disambiguate the arguments and the body.

The previous form that looked like that is a special case and still exists. The change is that it applies to modules with one arguments only:

  module Content_addressable (S : sig
    include Irmin.Content_addressable.S
  end) =
  struct
metanivek commented 1 year ago

I'm good with this. Thanks again for the draft PR for feedback!

Julow commented 1 year ago

OCamlformat 0.26.0 has been released, this PR can be merged (not squashed due to .git-blame-ignore-revs) or closed if it's a bit too early to upgrade.