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

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

> I did have a nice simple solution that did not involve deleting that chunk but
> just changed a few lines. However I tried to avoid it initially because it was
> very hard to get test coverage of. And after a while I decided (in my opinion)
> that it should probably not be there at all.
>
> I can reintroduce the code and just fix the lines pertaining to bug 751605,
> but I couldn't find an elegant way to give the change test coverage without
> breaking the core ABI (which is bad given how much this bug needs fixing in
> precise too).

Was it actually evident that this chunk was ever a part of the problem?

As far as I can tell, that chunk only ever comes into operation where constrainNewWindowSize fails and returns a width or height that we can't accept. bug 751605 was about cases where constrainNewWindowSize failed and the window still ended up on the wrong monitor.

As mentioned, reverting that change didn't seem to have any effect.

What was the problem with the test coverage?

« Back to merge proposal