Code review comment for lp://qastaging/~compiz-team/compiz/compiz.tests_1042537.3

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

> My first throught is why oh why does
>
> compiz::config::impl::CCSSettingValueListWrapper start with CCS? This prefix
> is surely no longer needed.

You are correct. Shall I remove these here or in a follow up MP based on this one?

>
> Seems weird to me that you would have a magic length number instead of just
> calling strlen
> static const char *upgrade = "upgrade";
> static const unsigned int upgrade_str_len = 7; // why not initialize to
> strlen(upgrade)?

Could also work I guess.

>
> Oh, have I mentioned how much I hate C?

Careful. :)

>
> You don't need to include both gtest and gmock. <gmock/gmock.h> includes
> gtest.h.
>
> None of these comments are blockers to merging.

« Back to merge proposal