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

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

include/core/windowgeometry.h:

Comments in GeometrySaver class should be doxygen comments - either start them with '///' or '/**' if you're using block comments.

plugins/place/src/place.cpp
You have two instances in this file of code being commented with with a 'XXX' label. If you don't need the code, delete it. At the very least tell us why it's commented out. If you're not sure whether you need the code or not, you need to find out before landing the merge.

src/windowgeometry.cpp
You have a comment that says:

/* XXX: Do not allow geometry to be saved if window
 * has not been placed */

Is this something that needs to be fixed before this lands?

You also have this interesting logic within the push(...) method:

unsigned int useMask = mask & ~mMask;
mMask |= useMask;

Perhaps I'm being dense, but surely this is the same as "mMask |= mask" ?

review: Needs Fixing

« Back to merge proposal