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