Review Request: Kickoff: List newly installed applications

[Aaron Seigo]

-----------------------------------------------------------
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
> 
>