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