Review Request: Kickoff: List newly installed applications
Aaron Seigo
aseigo at kde.org
Fri Mar 7 21:51:00 CET 2008
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://mattr.info/r/267/#review261
-----------------------------------------------------------
+1 on the idea, i have some personal reservations about the implementation...
trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp
<http://mattr.info/r/267/#comment196>
is this the right place for this? should it be the applet's globalConfig()?
trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp
<http://mattr.info/r/267/#comment197>
sync() ... should probably be using the applet's globalConfig() and sending the configNeedsSaving signal.
trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp
<http://mattr.info/r/267/#comment198>
maybe it's just me, but a comment explaining what this method does exactly might be useful/helpful for those that comes afterwards.
it's unfortunate that the only way to know what the last apps we've seen are is to build a list of all installed apps. zoinks.
i wonder if it would be possible to add a timestamp or something like that to ksycoca for new application entries?
trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp
<http://mattr.info/r/267/#comment199>
if (!d->seenPrograms.contains(shortStorageId)) ?
i see that you're using it_find later below, however. it seems it stores the date it was found as the next entry in the string list?
would it be faster/more efficient to use a hash here instead? e.g. QMap<QString, QDate> or some such. it would probably make the code a lot more readable and maintainable?
the only challenge would be to read/write it from the config file, but again i really think it makes more sense to store this information in ksycoca for fast retrieval and shared storage.
*shrug*
- Aaron
On 2008-03-07 10:57:54, Stephan Binner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://mattr.info/r/267/
> -----------------------------------------------------------
>
> (Updated 2008-03-07 10:57:54)
>
>
> Review request for Plasma.
>
>
> Summary
> -------
>
> Port of the Kickoff/3 new application logic/display.
>
>
> Diffs
> -----
>
> trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h
> trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp
>
> Diff: http://mattr.info/r/267/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Stephan
>
>
More information about the Panel-devel
mailing list