Review Request: KDE Observatory (new plasmoid)

[Aaron J. Seigo]

On November 16, 2009, Sandro Andrade wrote:
> somehow stable and with some minor enhancements to be done I think a
> review it's quite necessary because this is my first plasmoid :)

some thoughts:

* it should be in kdereview/plasma/applets/ and added to the build system; 
i've done that now

* i see that it uses Qwt in one class. this is likely to diminish both the 
user base of it as well as it's "plasma-ness". does Plasma::SignalPlotter give 
you something similar? if not, then a check in the CMakeLists.txt file for Qwt 
is required

* the collectors really look like something that would be better as a 
DataEngine. putting data and visualization into one plugin is generally 
frowned upon in plasma. things like the periodic updates of the collectors are 
taken care of for you if you use a DataEngine, other widgets can more easily 
use the results, the data can be shared between multiple instances of the 
kdeobservatory widget and the widget would be more ready for remote widget 
use. perhaps not necessary for 4.4, but should definitely be considered for 
4.5.

* m_progressProxy should be a Plasma::Meter (i just made some relevant methods 
slots in Plasma::Meter to make that even easier)

* collectFinished is probably getting called more often than needed as its 
connected to multiple collectors; that means views and what not will get 
updated for collectors that don't have new data?

* calling collection[something] as in "if (pair.second && 
m_viewProviders[view])" will create an entry in the collection if it doesn't 
exist which is probably an unwanted (or at least unneeded) side effect; in 
these cases you probably want m_viewProviders.value(view)

* passing in a rect and the code in IViewProvider::createView looks fishy and 
indeed doesn't quite work; when KdeObservator::m_viewContainer changes 
geometry, the views won't change their geometry. i'd suggest either using a 
QGraphicsLayout or else putting an event filter on the view container and 
adjusting the geometry of the views when the view container gets a resize or 
related events.

* calls like m_right->nativeWidget()->setIcon are generally unecessary. use 
m_right->setIcon() i'd look through all the nativeWidget() calls and see which 
can be removed.

* on first update, it sat and spun for the better part of a minute writing to 
disk. subsequent starts up are good. i wonder if this is because each commit 
is added one at a time to the sqllite db instead of several at once? this is a 
show stopper for first use in any case. it also happens when adding more days 
to the context :/

* worse, it blocks the app while doing so. the db should either be updated 
incrementally with regular returns to the mainloop to prevent blocking the 
whole plasma application (e.g. plasma-desktop) or it should be done in a 
thread (probably preferred in this case)

* the icon button in the Configure KDE Observatory page seems to have some 
layout issues; here it overlaps the divider line at the bottom?

* sync'ing every minute might be a bit much for the default. 5 or 10 minutes 
should be plenty for a default?

* speaking of defaults, is showing just one day of operations really enough to 
give a decent and moral boosting overview? would the last week's worth of data 
be better? a lot of projects see sporadic but regular commits and "one day" 
may not be a whole lot of progress. plasma is active daily as are some of the 
other big projects like kdepim ... but not all are so lucky. this does make 
the db building problem worse, of course.

* why is "enable transition effects" an option?

* why are some view options in the global config options and others in the 
Views page?

* from a user's perspective, why is there a "global config options"? would 
another name be more useful?

* the automatic transitions are nice, but when trying to page through them 
with the left/right buttons it really gets in the way. perhaps a "play/pause" 
button could be added to the interface and automatically set it to "pause" 
when one of the arrow buttons are manually pressed? perhaps the transition 
could also pause when the mouse is placed over a graph; this would make the 
tooltips more easily reachable, as right now getting tooltips is a bit of a 
challenge when the animations are ongoing.

* it seems that whenever the data is updated, the "slide show" starts from the 
beginning again; it looks a bit odd to have it start all over again. could it 
simply keep going with the slideshow instead of restarting it?

* the scroll arrows feel "backwards": the right arrow should take me to the 
"next" item, which means things should scroll to the left not the right.

* it would be really nice if it were possible to aggregate some project data; 
e.g. "plasma" is really kdelibs/plasma + kdebase/runtime/plasma + 
kdebase/workspace/plasmas + kdeplasma-addons ... not a required feature for 
sure, but a nice one to have perhaps

* it would make the widget much more immediately approachable if there were 
some project presets shipped, such as ones for kdelibs, kdepim, kdegames, 
plasma, etc. do you have thoughts on how that would be best achieved?

ok, that's probably more than enough for now :)

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20091116/e43caf26/attachment.sig