Code review comment for lp:~smspillaz/compiz-core/compiz-core.decor_928173

On Thu, 9 Feb 2012, Daniel van Vugt wrote:

> Review: Approve
>
> Nice. It works and bug 928173 is no more. Two stylistic comments though...
>

Good to hear.

> 1. Shouldn't these:
> compiz::window::extents::Extents::Extents () {}
>
> compiz::window::extents::Extents::Extents (int left, int right, int top, int bottom) :
> ...
> instead be written as:
> namespace compiz::window::extents {
>
> Extents::Extents () {}
>
> Extents::Extents (int left, int right, int top, int bottom) :
> ...
> } // namespace compiz::window::extents
>
> 2. Unnecessary namespaces: Shouldn't "compiz::window::extents" simply be "compiz::window::extents"? Or even just "compiz"? Namespaces should really only change across different build targets. So that would mean core uses "compiz" and plugin Foo uses "compiz::Foo". Though, this point is purely opinion.
> --
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.decor_928173/+merge/91986
> You are the owner of lp:~smspillaz/compiz-core/compiz-core.decor_928173.
>