Review Request 120926: Remove redundant code from KRun::KRunPrivate::isPromptNeeded()

David Faure faure at kde.org
Fri Oct 31 20:09:51 UTC 2014


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


I would rather that this is all encapsulated, so callers don't have to worry about it.

(and Emmanuel seems to agree, in RR 120171).

This is the way it's done for many other things. The "dont show again" acts just as if the message box had been shown, it's a low-level detail that it wasn't shown. For the app, the setter enables the overall feature that means "prompt, or use the saved setting for the prompt".

- David Faure


On Oct. 31, 2014, 5:56 p.m., Arjun AK wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120926/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2014, 5:56 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> Since `setShowScriptExecutionPrompt()` is used to specify whether a prompt should be shown or not, we don't neither need to check it again nor do we need to call `setRunExecutable()` inside KIO. The caller is anyway forced to do something like:
> 
>     if (value == "alwaysAsk") {
>         run->setShowScriptExecutionPrompt(true);
>     } else {
>         run->setRunExecutables(value == "execute");
>     }
> 
> 
> Diffs
> -----
> 
>   src/widgets/krun.cpp c623b58 
>   src/widgets/krun_p.h 61660c0 
> 
> Diff: https://git.reviewboard.kde.org/r/120926/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141031/3aaa055a/attachment.html>


More information about the Kde-frameworks-devel mailing list