Code review comment for lp:~vanvugt/compiz/fix-980663-1041066

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Lots of clean-ups and renaming.

I disagree with the bi-state enumeration and think a boolean is clearer in cases like this where you can use it.

I disagree with the suggested approach to coverage where you have to put code in a static library to get coverage data. Getting coverage information should be transparently possible for all test cases. I'm surprised it's not done that way already, but we can improve that later. See also bug 1045665.

And I disagree with putting a single, single-use class in a separate library just to avoid recompiling it in the test cases. Clarity and a simpler source tree is more important.

« Back to merge proposal