<div dir="ltr">On Wed, Sep 4, 2013 at 12:43 PM, Stephen Kelly <span dir="ltr"><<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">David Faure wrote:<br>
<br>
> On Sept. 3, 2013, 8:35 p.m., Martin Klapetek wrote:<br>
>><br>
>> -----------------------------------------------------------<br>
<div>>> This is an automatically generated e-mail. To reply, visit:<br>
>> <a href="http://git.reviewboard.kde.org/r/112495/" target="_blank">http://git.reviewboard.kde.org/r/112495/</a><br>
</div>>> -----------------------------------------------------------<br>
>><br>
>> (Updated Sept. 3, 2013, 8:35 p.m.)<br>
<div>>><br>
>><br>
>> Review request for KDE Frameworks.<br>
>><br>
>><br>
<br>
</div>1) It would be much better if you made several patches for things like this.<br>
One patch to remove the remove_definitions() calls, one patch to add<br>
Qt5::Gui to the link interface, one patch to add the needed<br>
find_dependency() calls, and then finally the patch moving the code without<br>
needing to change it. Reviewers should look for this stuff imo.<br></blockquote><div><br></div><div>Fair enough, though these changes were really small (no code needed to be changed with removing definitions, one line for find_dependency() etc...).</div>


<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
2) Why did you add Qt5::Gui to the link interface? As far as I can see it<br>
does not need to be.<br></blockquote><div><br></div><div>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.</div><div>


<br></div></div><div>Cheers</div>-- <br><div><span style="color:rgb(102,102,102)">Martin Klapetek | KDE Developer</span></div>
</div></div>