Re: automatic formatting checks for pull requests?

i have tested running clang-format against the entire php-src codebase, and there is only 1 file it breaks: ext/spl/spl_directory_arginfo.h more details at https://github.com/php/php-src/pull/13417#issuecomment-1950920114 On Sun, 18 Feb 2024 at 01:45, Derick Rethans <derick@php.net> wrote: > > On 17 February 2024 22:18:05 GMT, Ilija Tovilo <tovilo.ilija@gmail.com> wrote: > >Hi Hans > > > >On Sat, Feb 17, 2024 at 3:31 PM Gina P. Banyard <internals@gpb.moe> wrote: > >> > >> On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan <hans@loltek.net> > >> wrote: > >> > >> > Can we add automatic formatting checks for pull requests? > >> > Made a PR: > >> > https://github.com/php/php-src/pull/13417 > >> > >> It would be nice to have some formatting rules to harmonize the codebase as it is > >> somewhat the wild west, > >> but as far as my understanding goes is that Clang format struggles to understand our > >> codebase (namely macros) and is difficult to set-up for php-src. > > > >Right. Consistent code style is nice, but what we have now is really > >not that bad. There are a couple things I'd want if we enforce code > >style: > > > >* Fixing the style should be easy, running a single command without > >first pushing to CI. > >* It should be fast too, so that I can easily run it for every commit, > >preferably even on-save in my editor. > >* The new code style should be applied only to newly added sections or > >changed code, not entire files. Otherwise, we'll have many changes in > >large files, with endless merge conflicts when merging up from lower > >branches. > >* The formatting tool should work for all php-src code, not just plain > >C code. We don't want to be forced to refactor old macros just because > >we need to add a single line to some long-standing code. Last time I > >tried clang-format, it utterly failed with our macros. > > > >I haven't looked at your PR in detail, so I'm not sure which of these > >points it satisfies. It would be great if you could quickly describe > >how it works, and what the goals are. > > > >Essentially, I'm just sceptical that this isn't more trouble than it's > >worth. > > > >Ilija > > IMO, clang-format isn't really suitable. Its untunable style is often far from the coding > style that we currently have, and it makes some really odd choices as to when and how to wrap lines, > making code definitely less readable. > > cheers > Derick > IMO, clang-format isn't really suitable. Its untunable style is often far from the coding > style that we currently have, and it makes some really odd choices as to when and how to wrap lines, > making code definitely less readable. it is far from "untunable", the manpage for tuning is huge: https://clang.llvm.org/docs/ClangFormatStyleOptions.html