[Development] Proposal: (re)move qt5.git/_clang-format

[Volker Hilsheimer]

On 13 Sep 2023, at 13:23, Ahmad Samir <a.samirh78 at gmail.com> wrote:

On 13/9/23 11:06, Ivan Solovev via Development wrote:
I would therefore propose to remove the file from qt5.git:
+1 from my side.
I believe I simply do not have the clang-format tool installed on my system,
because it usually breaks the formatting of the patches, not improves them.
One way to address these problems, especially for new devs or drive-by
contributions, is stating clearly in the wiki page:
"use clang-format, it should get you at least half way there, but you still
should/must also override it to match the style of surrounding code, as much as
possible, in the file(s) you're editing, that's kinder to reviewers".
The problem is that we have a pre-commit (I believe) git-hook, which checks the
formatting and nags if it does not match to what we have in the _clang-format file.
This basically forces​ the new or drive-by contributors to submit the patch with an
incorrect formatting, which then leads to a bunch of review comments about code-style.
I think that it's better to give no hints, rather than misleading hints.

[...]

I think the best way to find out is indeed removing the file, then one of two things will happen:
- we'll get less formatting issues in reviews (especially from new contributors)
- or we'll still get formatting issues, just different ones than what clang-format currently does

So 6-12 months, and we'll have a definite answer. :)

Regards,
Ahmad Samir

I find the input from clang-format’s sometimes helpful, sometimes annoying. And the source of the annoyance is not that clang-format points out things that I could have formatted differently, but that our pre-commit hook returns with a non-zero exit code if clang-format produces a diff, thus blocking the commit.

Based on that, I’d propose to change the pre-commit hook: https://codereview.qt-project.org/c/qt/qtrepotools/+/503746

and leave the _clang_format file in place as a tool, not as a straight-jacket.

Volker


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20230913/2ce646e4/attachment-0001.htm>