cgiboudeaux added a comment.
In D15070#344900 <https://phabricator.kde.org/D15070#344900>, @bruns wrote: > In D15070#344884 <https://phabricator.kde.org/D15070#344884>, @cgiboudeaux wrote: > > > In D15070#344871 <https://phabricator.kde.org/D15070#344871>, @bruns wrote: > > > > > As all the raised concerns have been dealed with, can we give this a try while the next KF release is still somewhat in the future? > > > > > > No, the empty if must be removed. Code that does nothing is useless. > > > Its not empty, there is a comment inside. Of course I can add a `set(KDE_INSTALL_PYTHON${pyversion}DIR ${KDE_INSTALL_PYTHON${pyversion}DIR})` if you insist ... > > And if you restructure it you either end up with a lengthy if condition - `if (NOT KDE_INSTALL_PYTHON${pyversion}DIR AND KDE_INSTALL_USE_PYTHON${pyversion}_SYS_PATHS)` - or another nesting level. Both are significantly harder to read. If KDE_INSTALL_USE_PYTHON${pyversion}_SYS_PATHS is true, KDE_INSTALL_PYTHON${pyversion}DIR can be ignored. This was in the review REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D15070 To: bruns, #frameworks Cc: cgiboudeaux, bcooksley, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns
