[Kde-pim] Review Request 111541: Store only filename instead of full file path to part external payload file

[Volker Krause]

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

Ship it!


Looks good, thanks! Just some ideas for simplifications.


server/src/handler/fetchhelper.cpp
<http://git.reviewboard.kde.org/r/111541/#comment26716>

    Wouldn't mConnection->capabilities()->noPayloadPath() be enough here? probably even making the changes in the capability aggregator unnecessary.
    
    Also, I'm wondering if QFileInfo::isAbsolute() isn't faster/safer here, prepending the full storage path will fail in that case anyway, but things will continue to work if your part table content is really messed up and located all over the place (for whatever reason).



server/src/storage/parthelper.cpp
<http://git.reviewboard.kde.org/r/111541/#comment26717>

    another possible candidate for isAbsolute() instead, as well as in the methods below



server/src/storage/parthelper.cpp
<http://git.reviewboard.kde.org/r/111541/#comment26722>

    that 3 line pattern is repeated quite a few times, maybe factor out in a little helper method? resolveAbsolutePath or something like that


- Volker Krause


On July 16, 2013, 9:27 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111541/
> -----------------------------------------------------------
> 
> (Updated July 16, 2013, 9:27 p.m.)
> 
> 
> Review request for Akonadi and Volker Krause.
> 
> 
> Description
> -------
> 
> Store only filename instead of whole path in parttable. The path is always the same and we can easily reconstruct it in the server.
> 
> 
> Diffs
> -----
> 
>   libs/protocol_p.h a351227 
>   server/src/clientcapabilities.h f22795e 
>   server/src/clientcapabilities.cpp 0fc76ed 
>   server/src/clientcapabilityaggregator.h 01087a2 
>   server/src/clientcapabilityaggregator.cpp 4d90f8f 
>   server/src/handler/capability.h 3fb348b 
>   server/src/handler/capability.cpp 7052d1c 
>   server/src/handler/fetchhelper.cpp 90a89e4 
>   server/src/storage/parthelper.cpp 81eb4a0 
>   server/tests/unittest/parthelpertest.cpp 9b78edb 
> 
> Diff: http://git.reviewboard.kde.org/r/111541/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/