Review Request 114217: ebn krazy fixes in JuK

Michael Pyne mpyne at kde.org
Sat Nov 30 03:26:41 GMT 2013


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

Ship it!


These are all kind of micro-optimizations, but then again you did say 'krazy fixes', which is what these normally are. :)

See the notes though, especially for the qBound().


mpris2/mediaplayer2player.cpp
<http://git.reviewboard.kde.org/r/114217/#comment32070>

    KUrl doesn't actually have a .toEncoded() method, it inherits the one from QUrl, so I don't think this is actually appreciably different. Of course by the same token there's nothing wrong with using KUrl here either, except perhaps for the little bit of addition constructor/destructor time.



volumepopupbutton.cpp
<http://git.reviewboard.kde.org/r/114217/#comment32069>

    Instead of using float(0.0) you can use 0.0f (note the suffix) which lets the compiler know that the number is supposed to be of type float instead of double. This would reduce the number of unsightly casts while still fixing compiler warnings.


- Michael Pyne


On Nov. 29, 2013, 9:56 p.m., Shubham Chaudhary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114217/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2013, 9:56 p.m.)
> 
> 
> Review request for KDE Multimedia and Michael Pyne.
> 
> 
> Repository: juk
> 
> 
> Description
> -------
> 
> Changes: 
> * single quote chars
> * casted floats to qBound
> * KUrl instead of QUrl
> * removed empty spaces
> * added newline at eof
> 
> 
> Diffs
> -----
> 
>   lyricswidget.cpp 7bb591a 
>   mpris2/mediaplayer2player.cpp fe9b6b7 
>   scrobbleconfigdlg.h 8be1589 
>   scrobbler.cpp 0c5a5b9 
>   volumepopupbutton.cpp 5ac5e93 
> 
> Diff: http://git.reviewboard.kde.org/r/114217/diff/
> 
> 
> Testing
> -------
> 
> build and run
> 
> 
> Thanks,
> 
> Shubham Chaudhary
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20131130/be82f7fc/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia


More information about the kde-multimedia mailing list