qt: Prefix makefile variables with QT_ #23242

pull promag wants to merge 1 commits into bitcoin:master from promag:2021-10-qt-makefile changing 1 files +7 −7

promag commented at 11:44 AM on October 9, 2021: member

Improves consistency and readability if future QML variables are added.
promag cross-referenced this on Oct 9, 2021 from issue Prefix makefile variables with QT_ by promag
hebasto commented at 11:57 AM on October 9, 2021: member

Concept ACK. Use scripted-diff?
DrahtBot added the label Build system on Oct 9, 2021
DrahtBot added the label GUI on Oct 9, 2021
shaavan commented at 1:39 PM on October 9, 2021: contributor

Concept ACK

This PR focuses on improving the consistency and readability of Makefile.qt.include by renaming the variable per their proper function and usage. Since there is a high probability of introducing new QML variables in the future, this is a small but necessary change.

As we are on the matter of variable naming consistency, I want to add something, though. In the Makefile.qt.include on lines 292 and 324. Shouldn’t the variable BITCOIN_RS also be renamed to BITCOIN_QT_RS?
promag commented at 1:40 PM on October 9, 2021: member

I’ll take a look @shaavan, thanks.
laanwj commented at 12:21 PM on October 13, 2021: member

No objections, this probably improves the variable naming. Checked that none of the old names remain.

Code review ACK e8b6e7ccb45c0189ccc953589b8083a7fe0c56a8
hebasto approved
hebasto commented at 8:06 PM on October 13, 2021: member

ACK e8b6e7ccb45c0189ccc953589b8083a7fe0c56a8
promag force-pushed on Oct 13, 2021

scripted-diff: Prefix makefile variables with QT_

Improves consistency and readability if future QML variables are added.

-BEGIN VERIFY SCRIPT-
sed -i \
    -e 's/RES_ANIMATION/QT_RES_ANIMATION/g' \
    -e 's/RES_FONTS/QT_RES_FONTS/g' \
    -e 's/RES_ICONS/QT_RES_ICONS/g' \
    -e 's/BITCOIN_RC/BITCOIN_QT_RC/g' \
    src/Makefile.qt.include
-END VERIFY SCRIPT-

eb04badcd6

promag force-pushed on Oct 13, 2021
hebasto approved
hebasto commented at 8:32 PM on October 13, 2021: member

re-ACK eb04badcd6108e17c8ec78e9c316775c334030cb, only suggested changes, and script-diff used.
shaavan approved
shaavan commented at 11:31 AM on October 14, 2021: contributor
Code Review ACK eb04badcd6108e17c8ec78e9c316775c334030cb

Changes since my last review:
- Renamed BITCOIN_RC -> BITCOIN_QT_RC
- Used scripted-diff in commit message
MarcoFalke merged this on Oct 14, 2021
MarcoFalke closed this on Oct 14, 2021
promag deleted the branch on Oct 14, 2021
sidhujag referenced this in commit c0030feb82 on Oct 14, 2021
hebasto cross-referenced this on Oct 18, 2021 from issue Sync with the main repo by hebasto
hebasto referenced this in commit 13a95074e2 on Oct 19, 2021
barton2526 cross-referenced this on Jul 19, 2022 from issue qt: Prefix makefile variables with QT_ by barton2526
bitcoin locked this on Oct 30, 2022

Contributors

Labels