Review Request 112495: Move kcrash to tier2
Stephen Kelly
steveire at gmail.com
Wed Sep 4 11:08:47 UTC 2013
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.
More information about the Kde-frameworks-devel
mailing list