On 13 Sep 2023, at 16:25, Volker Hilsheimer via Development 
<development@qt-project.org> wrote:

On 13 Sep 2023, at 13:23, Ahmad Samir <a.samir...@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

I’ve now submitted the change, thanks for the reviews.


Volker

-- 
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development

Reply via email to