Compiz Core

Merge lp://qastaging/~smspillaz/compiz-core/compiz-core.tests_915950 into lp://qastaging/compiz-core/0.9.5

  1. compiz-core.tests_915950
  2. Merge into 0.9.5
Proposed by Sam Spilsbury
Status: Merged
Merged at revision: 2979
Proposed branch: lp://qastaging/~smspillaz/compiz-core/compiz-core.tests_915950
Merge into: lp://qastaging/compiz-core/0.9.5
Diff against target: 1328 lines (+749/-285)
9 files modified
plugins/place/CMakeLists.txt (+4/-1)
plugins/place/src/place.cpp (+126/-280)
plugins/place/src/place.h (+8/-4)
plugins/place/src/smart/CMakeLists.txt (+62/-0)
plugins/place/src/smart/include/smart.h (+73/-0)
plugins/place/src/smart/src/smart.cpp (+199/-0)
plugins/place/src/smart/tests/CMakeLists.txt (+14/-0)
plugins/place/src/smart/tests/offscreen/src/test-place-smart-on-screen.cpp (+156/-0)
plugins/place/src/smart/tests/offscreen/src/test-place-smart-onscren.cpp (+107/-0)
To merge this branch: bzr merge lp://qastaging/~smspillaz/compiz-core/compiz-core.tests_915950
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
Alan Griffiths Approve
Review via email: mp+90751@code.qastaging.launchpad.net

Description of the change

Tests that smart placement does not put windows offscreen on two different sized monitors

To post a comment you must log in.
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Whoops, missing a commit here

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

Or rather, there's a useless file here.

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

Maybe it's just me, but I'm concerned about the amount of core code being rewritten here (and wasn't reviewed). If it were up to me I wouldn't modify so much core code so close to release. I don't think all the changes are relevant to writing test cases either.

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

Sam reverted the merge. Needs review again.

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

Code was unintentionally pushed to trunk. thats now reverted.

As for this change - all it does is add an interface for a placeable object. the diff is large because things moved around etc

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

This *looks* OK, but like Daniel says it looks like several (sequential) changes overlayed on each other which makes it hard to validate.

I'd prefer to see separate commits for refactoring (moving/renaming stuff) and functional changes (making it work). (But I'm as guilty of mixing the two.)

review: Approve
Revision history for this message
Sam Spilsbury (smspillaz) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches