Re: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add 'make format' and remove tabs

[Julien Grall <julien@xxxxxxx>]

Hi,

On 08/11/2022 15:33, Edwin Török wrote:
> See CODING_STYLE: Xen uses spaces, not tabs.
>
> * OCaml code:
>
> Using `ocp-indent` for now to just make minimal modifications in
> tabs vs spaces and get the right indentation.
> We can introduce `ocamlformat` later.
>
> * C stubs:
>
> just replace tabs with spaces now, using `indent` or `clang-format`
> would change code too much for 4.17.
>
> This avoids perpetuating a formatting style that is inconsistent with
> the rest of Xen, and that makes preparing and submitting patches more
> difficult (OCaml indentation tools usually only support spaces, not tabs).
>
> Contains a bugfix for `abi-check` script to handle the change in the
> amount of whitespace.
>
> No functional change.
>
> Signed-off-by: Edwin Török <edvin.torok@xxxxxxxxxx>
>
> --
> Reason for inclusion in 4.17:
> - makes it easier to backport changes from master to 4.17
Right, but you will have the problem when backporting to 4.16 and older. 
So the overhead will always be there for a couple of years.
> - avoid perpetuating a different coding style (I thought tabs were
>    mandated by Xen, and was about to fix up my editor config to match
>    when I realized Xen already mandates the use of spaces)
> - should make submitting patches for OCaml easier (OCaml indentation
>    tools know only about spaces, so I either can't use them, or have to
>    manually adjust indentation every time I submit a patch)
> - it can be verified that the only change here is the Makefile change
>    for the new rule, 'git log -p -1 -w' should be otherwise empty
While I understand the goal and support, this seems to be a bit too late 
to do it in Xen 4.17 (we are only a couple of weeks away). At this stage 
of the release we should only do bug fix.
This is clearly only a comesmetic change and there I would argue this 
should be deferred to 4.18. That said the last call is from the RM.
Cheers,

--
Julien Grall