Review Request: Achievements 10 fixup

[Laszlo Papp]

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



player/lib/models/achievementsmodel.cpp
<http://git.reviewboard.kde.org/r/102569/#comment5725>

    Metadata is not documented for the constructor properly so that we need a proper pass of the API user here.
    
    There are more ways behind the documentation to handle this:
    
    1) assert. We did at the other place for some  cases. 
    
    2) silently not return any proper icon path, 
    
    I do not like the first two ones, and I would probably go fro this third one:
    
    3) Using a proper qDebug() error message below
    
    I could probably add a constructor expecting a projectFilePath arguement would also be a convinience in some cases, but that is not the scope of this patch. I can do that later, if the need arises.



player/lib/models/achievementsmodel.cpp
<http://git.reviewboard.kde.org/r/102569/#comment5726>

    The second problem is that, the d->metaData member is actually "leaking" now since it is not removed in the destructor and up to the API user what will happen with that.
    
    One way is that to have the metaData copy here (which will be a more straight-forward decision after the projectFilePath() based constructor) and delete it in the desctructor. That way it would not be deleted from the caller "silently".
    
    Also, we might need to document that how it happens on the API interface documentation (ie.: this will be internally deleted at the class destruction).


- Laszlo


On Sept. 10, 2011, 4:30 p.m., Felix Rohrbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102569/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2011, 4:30 p.m.)
> 
> 
> Review request for Gluon and Laszlo Papp.
> 
> 
> Summary
> -------
> 
> Fix showing the achievement icons in the kdeext player.
> 
> 
> Diffs
> -----
> 
>   player/lib/models/achievementsmodel.cpp 7f66f46 
> 
> Diff: http://git.reviewboard.kde.org/r/102569/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Felix
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gluon/attachments/20110912/7fed3d01/attachment.html>