Code review comment for lp://qastaging/~3v1n0/unity/lim

44 +const std::string SETTING_NAME("com.canonical.indicator.appmenu");
45 +const std::string SET
TING_KEY("menu-mode");

Maybe they shuld be in a unnamed namespace?

---

48 + : Indicator(name),
49 + gsettings_(g_settings_new(SETTING_NAME.c_str())),
50 + integrated_(false)

For the next time... :

48 + : Indicator(name)
49 + , gsettings_(g_settings_new(SETTING_NAME.c_str()))
50 + , integrated_(false)

---

setting_changed_.Connect(gsettings_, "changed::menu-mode", [&] (GSettings*, gchar* key) {
53 + CheckSettingValue();
54 + });

If you're not using the argument gchar* key, please use an unnamed variable too...

---

g_idle_add_full (G_PRIORITY_DEFAULT, (GSourceFunc) send_show_entry, data, NULL);
g_variant_get (parameters, "(s(iiuu))", &entry_name, &geo.x, &geo.y, &geo.width, &geo.height);

Remove the whitespace between the function name and the bracket.

---

AppmenuIndicator *appmenu = dynamic_cast<AppmenuIndicator*>(indicator.get());

:(

---

751 - * Copyright (C) 2010 Canonical Ltd
752 + * Copyright (C) 2012 Canonical Ltd

Please do 2010-2012

---

760 +#include <NuxCore/Rect.h>

I think you can use a forward declaration.

---

+ bool IsVisible();

Const?

---+ bool IsVisible();

Btw all of these are not blocking ;)