Merge lp://qastaging/~uriboni/compiz/unminimize-configurable-independently into lp://qastaging/compiz/0.9.8

Looks fine.

A side note that we really have to get rid of this in the animation plugin:

225 bool usedO, bool usedC, bool usedM,
226 - bool usedS, bool usedF,
227 + bool usedS, bool usedU, bool usedF,

256 - false, false, false, false, true,
257 + false, false, false, false, false, true,

UGH!

I have no idea what we'll replace it with though. I would say flags but my books on style guide say "don't do that". Then again, I guess function names here does not make sense either.

Any thoughts ?

uriboni, great idea to enhance the animation plug-in with configurable unminimize as well.

+1

Hmm, the abstraction in this case looks okay. I think since we are only disabling animations for shading / focusing you can probably have AnimationUsedFor::allExecptShade AnimationUsedFor::allExeceptShadeAndFocus

I won't block on that though.

I am not requiring tests here since :
 a) This is a relatively broad area to test
 b) Tests that cover just this code would not really check that the feature works (there is no "logic")
 c) Requiring tests here would hold up development of an otherwise needed feature.

> Hmm, the abstraction in this case looks okay. I think since we are only
> disabling animations for shading / focusing you can probably have
> AnimationUsedFor::allExecptShade AnimationUsedFor::allExeceptShadeAndFocus
>
> I won't block on that though.

There are a few cases where it's only focus or only shade that is not enabled, and in general I think it's better to leave the flexibility.
Perhaps one possible improvement may be to have something like:

AnimationEffectUsedFor::all().exclude(FocusAnimation).exclude(ShadeAnimation)
or
AnimationEffectUsedFor::none().include(ShadeAnimation)

Or something like this where exclude() and include() return a reference to the AnimationEffectUsedFor object so you can chain them and pass them directly to the parameter.

But I think it's good this way too.

> > Hmm, the abstraction in this case looks okay. I think since we are only
> > disabling animations for shading / focusing you can probably have
> > AnimationUsedFor::allExecptShade AnimationUsedFor::allExeceptShadeAndFocus
> >
> > I won't block on that though.
>
> There are a few cases where it's only focus or only shade that is not enabled,
> and in general I think it's better to leave the flexibility.
> Perhaps one possible improvement may be to have something like:
>
> AnimationEffectUsedFor::all().exclude(FocusAnimation).exclude(ShadeAnimation)
> or
> AnimationEffectUsedFor::none().include(ShadeAnimation)
>
> Or something like this where exclude() and include() return a reference to the
> AnimationEffectUsedFor object so you can chain them and pass them directly to
> the parameter.

In fact, I was thinking of something exactly like this, however I didn't know how hard it would be. If you think it would be easy, feel free to do it, otherwise I'm happy with this as it is :)

>
> But I think it's good this way too.

No commit message specified.

added commit message and approved again.

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-compiz-core/51/console reported an error when processing this lp:~uriboni/compiz/unminimize-configurable-independently branch.
Not merging it.

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-compiz-core/51/console reported an error when processing this lp:~uriboni/compiz/unminimize-configurable-independently branch.
Not merging it.