Re: [PATCH v4 06/11] tools/ocaml: add .clang-format

[Edwin Torok <edwin.torok@xxxxxxxxx>]

> On 19 Dec 2022, at 12:03, Anthony PERARD <anthony.perard@xxxxxxxxxx> wrote:
> 
> On Fri, Dec 16, 2022 at 06:25:15PM +0000, Edwin Török wrote:
>> Add a .clang-format configuration that tries to match CODING_STYLE where
>> possible.
> 
> Is there going to be a patch applying this to "tools/ocaml", like you
> did with "make format"?

Long term probably, not yet.

For now I just want to experiment with it to see how well and consistently it 
works (especially with different versions of the tool).
(I've been using ocp-indent and ocamlformat for years now, clang-format would 
be a brand new addition.
 There are some solutions for ocamlformat that might be useful for backports, 
e.g. there is a git merge driver
that reformats all 3 sides of a merge and then attempt to solve conflict/do the 
merge on the resulting formatted files,
so even if old code was formatted differently or with a different version it 
might be able to apply the changes.
I've yet to try it though, and I'm not yet sure how easy it would be to 
integrate something like that with 'guilt' or 'stgit'
which is what Xen uses to manage its patch-queue for backports, at least 
internally.

If there was an ocp-indent equivalent for C that just re-indents that might be 
something we could apply right now,
but I'm not aware of one.
)


>> I was not able to expr
>> ess the special casing of braces after 'do'
>> though, this can only be controlled generally for all control
>> statements.
>> It is imperfect, but should be better than the existing bindings, which
>> do not follow Xen coding style.
> 
> There isn't a single CODING_STYLE for all code in the repo so it isn't
> an issue if the binding where having a different coding style. But
> having as few coding style as possible in the repo is probably helpful.

Indeed, having *a* coding style (and automated checking/formatting) is what I'm 
more interested in rather than which particular one.
The current one used by the bindings is not documented, and although I can 
guess at based on surrounding code,
that surrounding code is subtly different based on when it was written, so it 
seemed better to adopt what is already documented
in CODING_STYLE.


> 
> You could maybe add a CODING_STYLE in "tools/ocaml" to say that the
> coding style is running `make format` or `clang-format ...`. And if
> there other rules like how to name certain variable, that could go in
> this file as well.
> 


Having a way to easily run these formatters would be a plus. 
Perhaps extending one of the container images that Xen already has to include 
(a recent enough version of) ocp-indent and clang-format, and similarly for the 
.spec file it might be useful
to update its BuildRequires to bring in the necessary formatters to make it 
easier to use this in backports.

And then all that can be documented in a CODING_STYLE or 
CONTRIBUTING.txt/CONTRIBUTING.md file in tools/ocaml.

Best regards,
--Edwin