> 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.
>

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.


- 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
> 
>

Reply via email to