Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122417
Precedence: bulk
MIME-Version: 1.0
References: <CAK7R4ay5kE-PGkb2qn8xvjsY0nLfMGcGBTztSeAy_9=X_BwmZg@mail.gmail.com>
 <p2eUiHGGPvgqcEwgk9ISftxabO5Q5sxW6-Eoz-LrbRQBGAPorVnFPKgFaAMeLlfzVQ7Ikwr9vWDa7ctd8t9q_2spBvb2kLW-SyW869R2zQA=@gpb.moe>
In-Reply-To: <p2eUiHGGPvgqcEwgk9ISftxabO5Q5sxW6-Eoz-LrbRQBGAPorVnFPKgFaAMeLlfzVQ7Ikwr9vWDa7ctd8t9q_2spBvb2kLW-SyW869R2zQA=@gpb.moe>
Date: Sat, 17 Feb 2024 23:18:05 +0100
Message-ID: <CAPyj-LA8eDWBGqYcw7S0DTRZXN1=6HFOncwgki-ny7xYA9FnVw@mail.gmail.com>
Subject: Re: [PHP-DEV] automatic formatting checks for pull requests?
To: PHP Internals List <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: tovilo.ilija@gmail.com (Ilija Tovilo)

Hi Hans

On Sat, Feb 17, 2024 at 3:31=E2=80=AFPM Gina P. Banyard <internals@gpb.moe>=
 wrote:
>
> On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan <hans@loltek.n=
et> 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 und=
erstand 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 wort=
h.

Ilija