Merge lp:~bilalakhtar/unity/unhide-launcher-sc-fix-1002440 into lp:unity

[Bilal Akhtar]

The launcher will auto hide when installing a program from the software center now.

This branch fixes bug #1002440 by unhiding the launcher only when an animation can be completed successfully.

Merge request for the same bug fix to the Precise SRU branch is here: https://code.launchpad.net/~bilalakhtar/unity/5.0series-sru-software-center-integration-fixes

[Tim Penhey (thumper)]

Would it not make sense to have an early return?

It seems that you are going to create a new launcher icon even if you are not going to show the launcher.
Is this what you expect?

Also,
  (!path.empty() && !path.compare("software-center-agent"))
is not very readable... how about
  (path == "software-centre=agent")

review: Needs Fixing

[Bilal Akhtar (bilalakhtar)]

I've made the change, thanks! Another review would be appreciated :)

[Bilal Akhtar (bilalakhtar)]

I've moved the SRU part to a common branch, and updated the description with clarification.

[Tim Penhey (thumper)]

> if (path.empty() || path.compare("software-center-agent"))
> return;

This is just not easily understandable.

if (path != "software-center-agent")
    return

This does exactly the same thing, and doesn't require any boolean logic or double-checking the compare method.
Please change it.

review: Needs Fixing

[Bilal Akhtar (bilalakhtar)]

I've made the change, thanks!

[Marco Trevisan (Treviño) (3v1n0)]

+1

review: Approve

[Didier Roche-Tolomelli (didrocks)]

Can we get a test for that before it's merged?

[Bilal Akhtar (bilalakhtar)]

On Mon, Jun 18, 2012 at 8:49 AM, Didier Roche <email address hidden> wrote:
> Can we get a test for that before it's merged?

It's a very minor change. I sure can do that for this Quantal branch
merge, but can someone please review the Precise SRU branch merge [1]?
It contains an SRU regression fix, and it needs to be SRUed ASAP,
since it's blocking a couple of bugs on Software Center.

[1]: https://code.launchpad.net/~bilalakhtar/unity/5.0series-sru-software-center-integration-fixes/+merge/110214

[Brandon Schaefer (brandontschaefer)]

Looks goods +1.

Manual test are here:
https://code.launchpad.net/~gary-lasker/unity/software-center-launcher-integration-tests/+merge/116137

review: Approve

[Unity Merger (unity-merger)]

No commit message specified.

=== modified file 'launcher/LauncherController.cpp'
--- launcher/LauncherController.cpp	2012-06-15 02:03:31 +0000
+++ launcher/LauncherController.cpp	2012-06-18 03:21:26 +0000
@@ -476,6 +476,11 @@
       return;
   }
 
+  // Check if desktop file was supplied, or if it's set to SC's agent
+  // See https://bugs.launchpad.net/unity/+bug/1002440
+  if (path.empty() || path == "software-center-agent")
+    return;
+
   SoftwareCenterLauncherIcon::Ptr result = CreateSCLauncherIcon(path, aptdaemon_trans_id, icon_path);
 
   CurrentLauncher()->ForceReveal(true);