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

Revision history for this message
Tim Penhey (thumper) wrote :

My first throught is why oh why does

compiz::config::impl::CCSSettingValueListWrapper start with CCS? This prefix is surely no longer needed.

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)?

Oh, have I mentioned how much I hate C?

You don't need to include both gtest and gmock. <gmock/gmock.h> includes gtest.h.

None of these comments are blockers to merging.

review: Approve

« Back to merge proposal