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

[Ahmad Samir]

On 13/9/23 00:11, apoenitz wrote:
> On Tue, Sep 12, 2023 at 11:33:17PM +0300, Ahmad Samir wrote:
>> A config file that is 80-90% correct is better than nothing.
> 
> I disagree.
> 
> The result of such a thing is that people submit patches matching the config
> 100%, deviating from the wanted style by 10-20%. Numbers are arbitrary
> here, but the effect is that in practice even long-term contributors start
> to submit "wrong" auto-formatted patches causing needless review roundtrips.
> 

The alternative is patches with arbitrary formatting; that's not better.

> On top of that repeatedly discussion are started about adjusting the style to
> the tool, because "the tool can't be adjusted".

True, but style arguments arise on their own too, tool or no tool.

One way to stop the recurring discussions would be to reach a majority consensus 
about a clang-format config file, format the whole code base of each module in one 
go, then apply the formatting for each new commit, less discussions. Is that 
ideal? no, because reaching a consensus about coding style is not easy.

> 
>> For example I have copied that file to qtbase/.clang-format and changed that
>> `template <>` part, it's still useful for me to have the file there as I
>> could select some text and format only that part in e.g. KDE's Kate. I can
>> override the changes to conform to qtbase's coding style, but I don't have to
>> do it all manually.
> 
> It's not /that/ hard to follow the /one/ code style that's uses in one's main
> project. I have some sympathy for cases where people submit occasional patches
> in a side project with some odd style rules, but for the day-time project
> typing code in its preferred style should become second nature - independent
> of any possible quirks and oddities there.
> 

Examples:
- typos (no space):
  if (foo && bar){
     //
  }

- lengthy lines of code:

static QMetaObject::Connection connect(const QObject *sender, 
PointerToMemberFunction signal, const QObject *receiver, PointerToMemberFunction 
method, Qt::ConnectionType type = Qt::AutoConnection);

I could format it manually, probably two steps, or let the tool do it

- debating about where to break the line:
     bool inSenderThread = currentThreadId == 
QObjectPrivate::get(sender)->threadData.loadRelaxed()->threadId.loadRelaxed();

after the assign =, or after == or ...etc; or just let a tool do it, I can 
disagree and override it if it looks too bad.


> Andre'
> D
> PS:
> 
>> Drive-by suggestion: each module should also have a .git-blame-ignore-revs
>> file (name should be unified in all Qt repos), to put commit hashes that
>> git-blame should ignore. (See 'git blame --ignore-revs-file').
> 
> Why?
> 

Useful when using git-blame, e.g.:
- the commit that moved inline keyword to fix MinGW warnings in qstring.h, seeing 
any line from that commit in git blame isn't useful/informative
- Formatting a repo with clang-format, that commit isn't useful in git blame context

Regards,
Ahmad Samir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <http://lists.qt-project.org/pipermail/development/attachments/20230913/166ae3e4/attachment.sig>