-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101282/#review3158
-----------------------------------------------------------



kdialog/kdialog.cpp
<http://git.reviewboard.kde.org/r/101282/#comment2697>

    "refs". Plural :)
    
    Clean up the mistake the previous author made while you are at that line.



kdialog/kdialog.cpp
<http://git.reviewboard.kde.org/r/101282/#comment2696>

    Do not create a new KIconLoader, but use the global one.
    
    The other issue is the hardcoded size. While the example uses 32 pixels, it 
does not imply the class does not handle different sizes.
    
    But if the fallback does not work anyway, just forget about the changes in 
this section (as long as you don't want to find the cause for the bug). Sorry 
for the noise.


- Christoph


On May 6, 2011, 12:16 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101282/
> -----------------------------------------------------------
> 
> (Updated May 6, 2011, 12:16 p.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Summary
> -------
> 
> Since I use kdialog a lot, I wanted it to be able to set an icon for passive 
> Plasma notifications as well.
> I saw that the icon was hardcoded to dialog-information and tried to enable 
> it to parse the --icon command line argument and pass it to the dbus call 
> that triggers the notification.
> I could not test the patch however, since I do not (yet) have a full 
> development environment or its dependencies, and I do not even know if this 
> is the right approach, but I hope you can have a look at it.
> 
> 
> Diffs
> -----
> 
>   kdialog/kdialog.cpp b80ad9a 
> 
> Diff: http://git.reviewboard.kde.org/r/101282/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kai Uwe
> 
>

Reply via email to