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
OpenPGP_signature
Description: OpenPGP digital signature
-- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
