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

[Harald Sitter]

> On Sept. 29, 2016, 12:58 p.m., Harald Sitter wrote:
> > app/kipiinterface.cpp, line 371
> > <https://git.reviewboard.kde.org/r/129072/diff/1/?file=481508#file481508line371>
> >
> >     what's with the 5 seconds?
> 
> Jonathan Riddell wrote:
>     doesn't work without it, the menu gets reloaded before all the files are written to disk or something
> 
> Harald Sitter wrote:
>     needs a comment to explain that?! also if I was you I'd find out why exactly that is. oh actually I just saw why...
>     
>     more importantly however, if I have a slower disk than you or it is under load it might still not work.
>     
>     two options come to mind:
>     
>     a) once you get a dirchanged that triggers a wait-for-finished future (or some other construct) which loops on a 2 second sleep until it hasn't gotten another dirchanged signal in the past 2 seconds. Which is however slightly meh since the documentation of the signal says "However, the last change in the sequence of changes will always generate this signal." so maybe in addition you want to use fileChanged
>     
>     b) do a) but lightweight by having the slot record a boolean dirty property, and start a 2s timer, the timer triggers another slot which checks the boolean and restarts the timer if it is not false. poor man's concurrency if you will.

Still not fixed. You now delete the watcher after 1s. So you made this worse.


- Harald


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


On Oct. 3, 2016, 2:10 p.m., Jonathan Riddell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129072/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2016, 2:10 p.m.)
> 
> 
> Review request for Gwenview, Kipi-plugins, 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
> -----
> 
>   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/20161003/953cfa24/attachment.html>