Code review comment for lp:~attente/unity/1291461

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

Nice, awesome work...

Just one last fix, I'm not using precompiled headers here (as ccache seems to not play nicely with them), so this gives me compilation issues.

Can you just add these includes: http://paste.ubuntu.com/7362584/ ?

1095 + if (activate_panel_)
1096 + {
1097 + ActivateFirst();
1098 + activate_panel_ = false;
1099 + }

What's the reason for that?

Why not just calling ActivateFirst inside ActivatePanel (or better, replacing the content of ActivatePanel with the content of ActivateFirst, removing the latter)?
InspectKeyEvent, for the way it's used can just return always true.

From my tests I didn't notice any problem using http://pastebin.ubuntu.com/7362673/

863 + std::list<Accelerator::Ptr> accelerators_;

Really optional, but in general using std::vector works better when just iterating over elements as you do.

« Back to merge proposal