Review Request 125976: add an update() method

[Sebastian Kügler]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125976/#review88138
-----------------------------------------------------------


In general, I like the idea. We have to be a bit careful to keep the code maintainable though.

Please also remove the extraneous debug output.


src/kpackage/package.h (line 329)
<https://git.reviewboard.kde.org/r/125976/#comment60456>

    Capital If



src/kpackage/package.cpp (line 786)
<https://git.reviewboard.kde.org/r/125976/#comment60457>

    QLatin1Char('/') ?



src/kpackage/packagestructure.h (line 80)
<https://git.reviewboard.kde.org/r/125976/#comment60458>

    and an older version (what?)...?



src/kpackage/packagestructure.h (line 81)
<https://git.reviewboard.kde.org/r/125976/#comment60459>

    If



src/kpackage/private/packagejobthread.cpp (line 321)
<https://git.reviewboard.kde.org/r/125976/#comment60460>

    spaces around <<; why a qWarning without any explanation?



src/kpackage/private/packagejobthread.cpp (line 326)
<https://git.reviewboard.kde.org/r/125976/#comment60461>

    Would probably be good to give the installed path here as well. Also, "Impossible *to* remove"...



src/kpackage/private/packagejobthread_p.h (line 58)
<https://git.reviewboard.kde.org/r/125976/#comment60463>

    As you note yourself, perhaps just make it an enum? It's not private API, OK, but the code here is getting really convoluted that I think another boolean trap won't help...



src/kpackage/private/versionparser.cpp (line 25)
<https://git.reviewboard.kde.org/r/125976/#comment60462>

    I think we can remove most of these includes?


- Sebastian Kügler


On Nov. 6, 2015, 5:57 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125976/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2015, 5:57 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Kai Uwe Broulik.
> 
> 
> Repository: kpackage
> 
> 
> Description
> -------
> 
> new job based update() function that compared to install()
> if a package with the same pluginId is already installed,
> removes the old one before installing the new one, if
> and only if the version of the new one is more recent
> 
> 
> Diffs
> -----
> 
>   autotests/plasmoidpackagetest.h f730dce 
>   autotests/plasmoidpackagetest.cpp cccc567 
>   src/kpackage/CMakeLists.txt 3696f37 
>   src/kpackage/package.h 4ada8da 
>   src/kpackage/package.cpp 539b21a 
>   src/kpackage/packagestructure.h 9427b42 
>   src/kpackage/packagestructure.cpp 0070514 
>   src/kpackage/private/packagejob.cpp 0d2241b 
>   src/kpackage/private/packagejob_p.h 267429f 
>   src/kpackage/private/packagejobthread.cpp ca523b3 
>   src/kpackage/private/packagejobthread_p.h bf8a266 
>   src/kpackage/private/versionparser.cpp PRE-CREATION 
>   src/kpackagetool/CMakeLists.txt 78e0fb0 
> 
> Diff: https://git.reviewboard.kde.org/r/125976/diff/
> 
> 
> Testing
> -------
> 
> covered by autotests
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20151107/d7f7ba5f/attachment-0001.html>