Review Request 129072: install kipi-plugins in gwenview using appstream:/ URLs

[Aleix Pol Gonzalez]

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




app/kipiinterface.cpp (line 210)
<https://git.reviewboard.kde.org/r/129072/#comment67003>

    Use QPointer



app/kipiinterface.cpp (line 367)
<https://git.reviewboard.kde.org/r/129072/#comment67004>

    Maybe you'd want to delete the pluginwatcher here then?
    Or would it make sense to actually watch the directories regardless of whteher the install was triggered?
    
    Also endl is not needed in qDebug anymore.


Also I know it's quite some questions already, but would it make sense to get this code in upstream kipi? It's not something specific about gwenview anyway.

- Aleix Pol Gonzalez


On Sept. 29, 2016, 2:16 p.m., Jonathan Riddell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129072/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2016, 2:16 p.m.)
> 
> 
> Review request for Gwenview, Aleix Pol Gonzalez and Harald Sitter.
> 
> 
> Repository: gwenview
> 
> 
> Description
> -------
> 
> Alternative to patch https://git.reviewboard.kde.org/r/129004 for comparison, this uses an appstream:// URL to launch Plasma Discover (or whatever you have that handles those URLs) to install kipi-plugins.  A file watcher will update the menu when it spots the plugins have been installed.
> 
> As discussed on other review request, I prefer the built in approach as user experience.  Discover is quite a complex window that gets shown to the user and it may not be clear that the user needs to click install and wait then close Discover and check the menu again.  The simple modal dialog of the other patch seems easier to users.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 82b219a 
>   app/kipiinterface.h 1c06cd4 
>   app/kipiinterface.cpp 04dccb3 
> 
> Diff: https://git.reviewboard.kde.org/r/129072/diff/
> 
> 
> Testing
> -------
> 
> built it, tried to use discover to install kipi (reverted to using apt as it doesn't work on Neon currently) and noted the menu gets filled
> 
> 
> Thanks,
> 
> Jonathan Riddell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gwenview-devel/attachments/20160929/54e65a51/attachment.html>