Review Request 117175: Fix installing new .comic packages from GHNS to appear in the installed packages list in the comic widget.

Lamarque Souza lamarque at kde.org
Sun Mar 30 19:40:25 BST 2014



> On March 30, 2014, 11:23 a.m., Lamarque Souza wrote:
> > Hi, there is an error when trying to show the patch on reviewboard. Can you provide the correct patch?
> > 
> > I looked into the raw patch and I think the "return 1" line that you commented should be kept when the action is not upgrade.
> 
> Andrei Amuraritei wrote:
>     The case tested there are actions 'remove' or 'upgrade'. Since that gets called for a remove action there is a delete installer in case of failure to remove the package as in line 409 and 410.
>     For upgrade action if the "return 1" line is left then there is no install or upgrade action executed further down with the package.
>
> 
> Lamarque Souza wrote:
>     I agree that the code should not return when action is 'upgrade'. When action is 'remove' and nothing is actually removed (because there is no package to remove in this case) runKbuildsycoca() will be called unnecessarily some lines later, that is what I am trying to avoid. As long as calling runKbuildsycoca() here is ok for the other reviewers I am ok with your patch.
> 
> Andrei Amuraritei wrote:
>     runKbuildsycoca() doesn't get called unless there's a package specified with an action to execute. The check package.isEmpty() asures that.

The lines below are at the beginning of function the code in question resides:

    if (args->isSet("remove")) {
        package = args->getOption("remove");
    }

So package will not be empty when using 'remove' action, then runKbuildsycoca() will get called because the check package.isEmpty() will return false. Am I correct?


- Lamarque


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


On March 30, 2014, 1:13 p.m., Andrei Amuraritei wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117175/
> -----------------------------------------------------------
> 
> (Updated March 30, 2014, 1:13 p.m.)
> 
> 
> Review request for KDE Runtime, Albert Astals Cid, Aaron J. Seigo, Ian Monroe, and Lamarque Souza.
> 
> 
> Bugs: 306279 and 325028
>     http://bugs.kde.org/show_bug.cgi?id=306279
>     http://bugs.kde.org/show_bug.cgi?id=325028
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> -------
> 
> When installing a new .comic provider from GHNS, it doesn't appear in the installed list on the comic widget.
> This fixes it.
> 
> 
> Diffs
> -----
> 
>   plasma/tools/plasmapkg/main.cpp 61492fe 
> 
> Diff: https://git.reviewboard.kde.org/r/117175/diff/
> 
> 
> Testing
> -------
> 
> Compile, add new comic widget, install new comic providers.
> 
> 
> Thanks,
> 
> Andrei Amuraritei
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140330/953f526e/attachment.htm>


More information about the kde-core-devel mailing list