KCapacitybar on places item view

[Rafael Fernández López]

Hi Kevin and all,

> Now my comments on the patch itself.
>
> 1) It's unfortunately a no-go in its current form as I tested it and got
> the attached screenshot. Note really well placed I think. ;-)

This one is pretty strange and sounds like my first trying on this... maybe I 
generated wrongly the patch, or you had some garbage on it. Please, before 
applying this patch (note you will need patch -p1, since it is generated with 
git), make sure your local folder is clean.

I am sure this is something wrong with the patch or applying it, or if you did 
some special combination of actions, please specify. I am sure it is pretty 
impossible to get this with the current code.

> 2) Please don't move the KFilePlacesView::Private class inside the cpp
> file. The current layout is perfectly intended, I'd like to avoid coupling
> the delegate with the view class as much as possible. The idea being that
> at one point we could make the delegate more generic and help get the kind
> of animations available in the places delegate to other places. The
> coupling is already bad as it is, this move and the friend directive makes
> it worse. ++beer from me if you avoid it.

Avoided. I moved the maps to the delegate, and added some accessors (probably 
their names are not good, but you know, that is not my strongest ability). 
Suggestions for them ?

> 3) _k_timeLineValueChanged() should probably have another name. It's not
> the only time line used in there, so please make it more specific.

Fixed. Called now _k_capacityBarTimeLineValueChanged().

> 4) _k_slotPollDevices() should be renamed too. We don't use the "slot"
> convention in names for this class.

Fixed. Called now _k_pollDevices().

> 5) The indentation in KFilePlacesEventWatcher event filter should be
> modified to avoid the two aligned } at the end of each case block.

Fixed.

The patch can be found at:
http://media.ereslibre.es/2008/08/kdelibs.diff

Ok to commit ?


Regards,
Rafael Fernández López.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080806/6dfba29a/attachment.sig>