bitcoin_qt.m4: Minor fixes, improve code orthogonality #11101

pull lemzwerg wants to merge 0 commits into bitcoin:master from lemzwerg:master changing 0 files +0 −0

lemzwerg commented at 7:01 AM on August 21, 2017: contributor

Please use

git diff --word-diff-regex=. --word-diff=color

to view the differences.

The commit contains the following minor fixes.

. Only include '<QtCore/qconfig.h> for Qt5's QT_VERSION tests. For Qt4, also include '<QtCore/qglobal.h> for the QT_VERSION tests. This makes the tests work even if '-fPIC' or '-fPIE' is not used. . Change quotes from incorrect double quotes to brackets in 'BITCOIN_QT_CONFIGURE'. . Add some missing braces around variables in autoconf messages. . Add a missing dollar sign in '_BITCOIN_QT_IS_STATIC'.

Additionally, the patch does the following.

. Add double qoutes to string tests where arguments could (theoretically) contain spaces. . Remove double quotes where not necessary. . Consequently quote all arguments of M4 functions. . Mark empty M4 arguments with '[]'. . Other minor cosmetic fixes.
jonasschnelli commented at 7:11 AM on August 21, 2017: contributor

No sure if it's worth to touch this file with cosmetic changes. ping @theuni
jonasschnelli assigned theuni on Aug 21, 2017
practicalswift commented at 9:51 AM on August 21, 2017: contributor

@lemzwerg Could you split the changes into one commit each? It might be the case that you'll get ACK:s on some of the changes and NACK:s on some.
practicalswift commented at 10:00 AM on August 21, 2017: contributor

@lemzwerg Is there a linting tool that can find the issues you addressed in this PR?
practicalswift commented at 11:55 AM on August 21, 2017: contributor

@lemzwerg Could you investigate the OS X build failure? :-)
lemzwerg commented at 1:16 PM on August 22, 2017: contributor

@jonasschnelli, I can easily provide a complete formatted rewrite that avoids overlong lines, has a more vertical flow, and much more comments – I did this already in a private version, mainly to get completely accommodated to the file since I want to use it in one of my own projects. Just tell me whether I shall submit such a change (on top of the real buglets). @practicalswift, I will provide smaller commits soon. I think that I've also found the reason for the failure – I incorrectly assumed that the move of QT_VERSION from qglobal.h to qconfig.h happened with Qt 5.0 (it actually seems to have happened in Qt 5.4).
practicalswift commented at 9:37 PM on August 22, 2017: contributor

Concept ACK! Nice cleanups and thanks for splitting up the changes into separate commits!

Also: Werner, a huge thank you is hereby sent to you for the FreeType project and your >20 years of working on it! ❤️
lemzwerg commented at 4:31 AM on August 23, 2017: contributor

Thanks :-)
laanwj added the label Build system on Aug 23, 2017
lemzwerg closed this on Sep 3, 2017
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#11222 bitcoin_qt.m4: Minor fixes and clean-ups.