Review Request: Enable kdialog to set an icon for passivepopup as well

Christoph Feck christoph at maxiom.de
Tue May 10 11:10:55 BST 2011


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

Ship it!


Thanks for looking at the issue and fixing it. If you already have commit rights, please change the two issues below and commit to git master.

If you have not, this would be your next step :)


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

    "text" argument also needs a const reference.



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

    Please change these two lines to:
    
        passiveicon = KIconLoader::global()->loadIcon( ... );
    
    The temporary variable is unnecessary (and confusingly named).


- Christoph


On May 10, 2011, 9:58 a.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101282/
> -----------------------------------------------------------
> 
> (Updated May 10, 2011, 9:58 a.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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110510/262344b5/attachment.htm>


More information about the kde-core-devel mailing list