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

On 23/06/2013 7:00 PM, "MC Return" <email address hidden> wrote:
>
> What are all those "/P"s in the commit message about ?

Just a demonstration that each test is parameterized (so its not repeated 4
times)
>
> I think some word is missing in this comment (probably it is 'disabled'):
>
> 264 + /* The use of the timeout is currently because some
functionality
> 265 + * is broken which would cause these barrier conditions to
never
> 266 + * eventuate. For now just get to the fail condition */

Thanks.

>
> 187 + void DisallowDecorationsOnWindow (Window window);
>
> Maybe ForbidDecorationsOnWindow (Window window); would be better ? You
decide.

That works too.
>
> This comment could be a one-liner (ultraminor):
>
> 406 + /* Select for StructureNotify on the parent and wrapper
> 407 + * windows */
>
> 823 - unsigned int width, height, border, depth;
> 824 + unsigned int width, height, border;
>
> Don't we need depth anymore ? A quick search in the code shows that it
was not
> really used, although windows should have a depth for advanced
animations, no ?

"Depth" as in bit depth . the variable was removed as I replaced
XGetGeometry usage with a wrapper function which doesn't return a value for
that variable as i don't care about it.

>
>
> --
>
https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1188900.1/+merge/170954
> Your team Compiz Maintainers is requested to review the proposed merge of
lp:~compiz-team/compiz/compiz.fix_1188900.1 into lp:compiz.