Code review comment for lp://qastaging/~3v1n0/unity/switcher-glowing-decorations

Revision history for this message
Marco Trevisan (TreviƱo) (3v1n0) wrote :

> 3026 lines (+1085/-455) 26 files modified
>
> I hate you :)

:-P

> 1893 + typedef std::vector<LayoutWindow::Ptr> List;
>
> You call it List but it's a vector. I really don't like it.

It was already called List, so I kept the name it had... But I can change it to vector if you prefer.

> 868 + if (launcher->Hidden() || launcher_hide_mode == LAUNCHER_HIDE_NEVER
> || launcher_hide_mode == LAUNCHER_HIDE_AUTOHIDE)
>
> Does it makes sense? launcher_hide_mode can be LAUNCHER_HIDE_NEVER or
> LAUNCHER_HIDE_AUTOHIDE so this expression evaluates always true, right?

Yeah, you're right... I just did a replacement of the old variable, but this seems to be wrong, I guess that the logic should just be: continue if (launcher->options()->hide_mode == LAUNCHER_HIDE_AUTOHIDE && launcher->Hidden())... Fixing it.

> Please next time consider to have a different branch for style changes :)

You know, it was all mixed... It was hard to split.

« Back to merge proposal