Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122419
Date: Sun, 18 Feb 2024 00:00:55 +0000
To: internals@lists.php.net
Subject: Re: [PHP-DEV] automatic formatting checks for pull requests?
User-Agent: K-9 Mail for Android
In-Reply-To: <CAPyj-LA8eDWBGqYcw7S0DTRZXN1=6HFOncwgki-ny7xYA9FnVw@mail.gmail.com>
References: <CAK7R4ay5kE-PGkb2qn8xvjsY0nLfMGcGBTztSeAy_9=X_BwmZg@mail.gmail.com> <p2eUiHGGPvgqcEwgk9ISftxabO5Q5sxW6-Eoz-LrbRQBGAPorVnFPKgFaAMeLlfzVQ7Ikwr9vWDa7ctd8t9q_2spBvb2kLW-SyW869R2zQA=@gpb.moe> <CAPyj-LA8eDWBGqYcw7S0DTRZXN1=6HFOncwgki-ny7xYA9FnVw@mail.gmail.com>
Message-ID: <4EB2FF2E-84F9-4603-BEDC-7A359539FD97@php.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: quoted-printable
From: derick@php.net (Derick Rethans)

On 17 February 2024 22:18:05 GMT, Ilija Tovilo <tovilo=2Eilija@gmail=2Ecom>=
 wrote:
>Hi Hans
>
>On Sat, Feb 17, 2024 at 3:31=E2=80=AFPM Gina P=2E Banyard <internals@gpb=
=2Emoe> wrote:
>>
>> On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan <hans@loltek=
=2Enet> wrote:
>>
>> > Can we add automatic formatting checks for pull requests?
>> > Made a PR: https://github=2Ecom/php/php-src/pull/13417
>>
>> It would be nice to have some formatting rules to harmonize the codebas=
e as it is somewhat the wild west,
>> but as far as my understanding goes is that Clang format struggles to u=
nderstand our codebase (namely macros) and is difficult to set-up for php-s=
rc=2E
>
>Right=2E Consistent code style is nice, but what we have now is really
>not that bad=2E 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=2E
>* It should be fast too, so that I can easily run it for every commit,
>preferably even on-save in my editor=2E
>* The new code style should be applied only to newly added sections or
>changed code, not entire files=2E Otherwise, we'll have many changes in
>large files, with endless merge conflicts when merging up from lower
>branches=2E
>* The formatting tool should work for all php-src code, not just plain
>C code=2E 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=2E Last time I
>tried clang-format, it utterly failed with our macros=2E
>
>I haven't looked at your PR in detail, so I'm not sure which of these
>points it satisfies=2E It would be great if you could quickly describe
>how it works, and what the goals are=2E
>
>Essentially, I'm just sceptical that this isn't more trouble than it's wo=
rth=2E
>
>Ilija

IMO, clang-format isn't really suitable=2E Its untunable style is often fa=
r from the coding style that we currently have, and it makes some really od=
d choices as to when and how to wrap lines, making code definitely less rea=
dable=2E

cheers
Derick