Martin Klapetek wrote: >> 1) It would be much better if you made several patches for things like >> this. >> One patch to remove the remove_definitions() calls, one patch to add >> Qt5::Gui to the link interface, one patch to add the needed >> find_dependency() calls, and then finally the patch moving the code >> without needing to change it. Reviewers should look for this stuff imo. >> > > Fair enough, though these changes were really small (no code needed to be > changed with removing definitions, one line for find_dependency() etc...).
Good patches are indivisible. That makes them small, which is actually a good thing, not a bad thing as you imply. > >> >> 2) Why did you add Qt5::Gui to the link interface? As far as I can see it >> does not need to be. >> > > Because KCrash needs qwindowdefs.h, which is in QtGui and afaiu the link > interface is what sets include paths now, so without Gui it wouldn't > build. Actually it's the link implementation that provides the include paths. You put the Qt5::Gui in the LINK_PUBLIC section. It should be in the LINK_PRIVATE section. http://community.kde.org/Frameworks/Epics/kdelibs_cleanups/link_private_details Thanks, Steve. _______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
