Code review comment for lp://qastaging/~smspillaz/unity/unity.fix_864758

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

Hi Sam,

This class seems weird. Ideally we should try to avoid
global scope as much as possible, which includes static lists
if at all possible.

OMG... where to start with this. In order to gain context
about what was going on, I opened the complete file. That needs
to be fixed badly. There is a template class for absolutely
no reason, apart from adding complexity where we don't need it.

It is accessing a global variable called "screen" AFAICS.
Please tell me I'm wrong.

We have one class with two responsibilities. The static members
control the static data, and the member functions control the
minimisation of the window itself. This really needs to be
broken into two, kill the static methods, have the object
owned by the UnityScreen.

Also... why are these in the compiz namespace? That is just nuts.

About a gazillion lines of complicated code can be made simpler
here.

While this branch may well fix the bug, it doesn't make me happy.

review: Approve

« Back to merge proposal