Code review comment for lp:~alales/unity/unity-4.0-fix-for-791810

On Wed, 2011-11-30 at 08:16 +0000, Andrea Azzarone wrote:
> Review: Needs Fixing
>
> > On Sat, 2011-11-26 at 01:41 +0000, Andrea Azzarone wrote:
> > > Review: Needs Fixing
> > >
> > > 17 +Launcher::OnExpoTerminated()
> > > 18 +{
> > > 19 + _hide_machine->SetQuirk (LauncherHideMachine::EXPO_ACTIVE,
> > WindowManager::Default ()->IsExpoActive ());
> > >
> > > Plese don't put a space between the function name and the open parenthesis.
> >
> > OK, I've done this.
> >
> > >
> > > 20 + _hide_machine->SetQuirk (LauncherHideMachine::SCALE_ACTIVE,
> > WindowManager::Default ()->IsScaleActive ());
> > >
> > > Is the above line really needed? We're in the "OnExpoTerminated" function...
> >
> > Prior to my modification, this line was used in the method
> > Launcher::OnPluginStateChanged()
> >
> > That method was connected to the terminate_expo event
> >
> > My intention was to use the same code plus the call to
> > QuicklistManager::Default()->Activate() that I've added, just to make sure
> > I don't break any existing features...
>
> I know but you still have the function OnPluginStateChanged that is called when the scale/spread plugin state change.
>
OK, this is done.
> >
> > >
> > > 26 + if (_hidemode == LAUNCHER_HIDE_NEVER)
> > > 27 + return;
> > > 28 +}
> > >
> > > This if is not really needed. Please remove it.
> >
> > OK, I've done this.
> >
> > >
> > > 54 +void QuicklistManager::Activate()
> > > 55 +{
> > > 56 + if (_current_quicklist)
> > > 57 + _current_quicklist->Activate();
> > > 58 +}
> > >
> > > Wrong indentation. Please fix it :)
> >
> > OK, I've done this.
> >
> > >
> > > After the adjustments i'm going to continue the review. Thanks in advance :)
> >
> > OK. Thanks.