Code review comment for lp:~compiz-team/compiz/compiz.fix_1053280

> No obvious problems in testing. Just a few things:
>
> 1. These should be dropped now:
> Type type (); // BCI: drop this in 0.9.9
> If you need to constify functions as part of this proposal then no problem.
> Otherwise constification and other general clean-ups should be done as
> separate proposals.

Done. Constification was necessary.

>
> 2. BCI, PV? Are those acronyms explained in the source files they're
> mentioned?

BCI == Binary Compat
PV == Pure Virtual

>
> 3. I would much prefer that the namespace depth did not affect the indentation
> of declarations. I would accept:
> namespace a { namespace b {
> or
> namespace a {
> namespace b {
> Because namespaces should be changeable without affecting the surrounding
> code. So if the depth changed then you shouldn't ever need to re-indent your
> code.

Debatable but okay.

>
> 4. Two functions:
> PrivateAction::makeActive()
> PrivateAction::makeInactive()
> should be one:
> PrivateAction::setActive(bool active)
> Which will not only shrink the interface but is much more flexible for callers
> who want to change the setting based on a bool, like ca::setActionActiveState.
>

Okay

> 5. End-users of identifiers in namespaces like src/screen.cpp should use:
> using namespace compiz::actions;
> foo
> bar
> instead of:
> namespace ca = compiz::actions;
> ca::foo
> ca::bar
> It's cleaner to read, and again more flexible should namespaces ever change.

using is bad because it just imports namespace conflicts and defeats the purpose of namespaces. namespace aliases are designed to fix that problem. Also considering that we have implementation and interface namespaces the conflicts are a very real problem.