Review Request: Use the product as text for pluggable/removable devices.

[Frank Reininghaus]

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


Thanks for the patch! I'm looking forward to seeing meaningful device names in the Places Panel in the future :-)

Looks good, but I have a few little comments, see below.


dolphin/src/panels/places/placesitem.cpp
<http://git.reviewboard.kde.org/r/106639/#comment15661>

    We put the '*' before the space in Dolphin code.
    
    Moreover, I think you could move this part of your patch a bit further down, between  'setUdi(udi)' and the second part, right? Better keep things that belong together close to each other.



dolphin/src/panels/places/placesitem.cpp
<http://git.reviewboard.kde.org/r/106639/#comment15662>

    To make the overall structure of the code clearer, I would add an 'else' branch here and move the line 'setText(m_device.description())' from further up into that branch.


- Frank Reininghaus


On Sept. 29, 2012, 8:57 p.m., Alex Fiestas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106639/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2012, 8:57 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> Use product instead of description for pluggable/removable devices.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/panels/places/placesitem.cpp f72f639 
> 
> Diff: http://git.reviewboard.kde.org/r/106639/diff/
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> after
>   http://git.reviewboard.kde.org/r/106639/s/747/
> before
>   http://git.reviewboard.kde.org/r/106639/s/748/
> 
> 
> Thanks,
> 
> Alex Fiestas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20121001/aa0db2ad/attachment.htm>