Code review comment for lp:~uriboni/unity-2d/unity-2d-panel-pluggable

data/com.canonical.Unity2d.gschema.xml:

24 + <schema path="/com/canonical/unity-2d/panel/" id="com.canonical.Unity2d.Panel" gettext-domain="unity-2d">
25 + <key type="as" name="applets">
26 + <default>["!homebutton", "!separator", "appname", "!legacytray", "indicator"]</default>
27 + <summary>Applets to display in the panel</summary>

GSettings schema allows you to write this in a simpler way: you could use type="a(sb)" where "s" is the name and "b" is the boolean for "only on first screen". And if you need it, you can also add the expanding here, having the type as "a(sbb)". I see that there's also a "child" element for GSettings schema, but I don't know much about GSettings myself, so I'm not sure how suitable it is for our case.

34 + <key type="s" name="expanding">
35 + <default>"appname"</default>
36 + <summary>Applet that should expand to fill all unused space</summary>

Why does it need to be specified here? I'd rather let the applet itself set the desired policy on its widget; also, in that way you could have more than one applet expanding, and not just one.

libunity-2d-private/src/appletproviderinterface.h:
88 + virtual QString getAppletName() const = 0;
89 + virtual QWidget* getApplet() const = 0;

Qt coding style is appletName() and applet(), with no "get".
(it would also be "Type *variable", but I see that's never used in unity-2d so
I won't interfere :-) )

panel/CMakeLists.txt:
104 -add_subdirectory(tests)
105 +#add_subdirectory(tests)

Looks suspicious. :-)

panel/app/panelmanager.cpp:
162 -#include <QLabel>
163 +#include <QProcessEnvironment>
164 +#include <QFileInfo>
165 +#include <QDir>
166 +#include <QHash>
167 +#include <QPluginLoader>
168 +#include <QDebug>
169 +#include <QVariant>

Please sort include files alphabetically (and generally, any list where order is not otherwise important).

206 + if (!pluginFileInfo.exists() || !pluginFileInfo.isDir()) {
207 + qWarning() << "Panel plugin path does not exist or it is not a directory:" << pluginPath;
208 + return plugins;
209 + }

The "exists" test is unnecessary, and the error message can also simply be: "The plugin directory does not exist:".
BTW, I see the rest of the code is using UQ_WARNING and UQ_DEBUG instead of qWarning and qDebug.

226 + if (provider == NULL) {

Qt style prefers 0 to NULL.

296 + if (!(onlyLeftmost && screen != leftmost)) {
297 + QWidget *applet = provider->getApplet();

This condition's logic is a bit twisted. :-)
I suggest:
if (screen == leftmost || !onlyLeftmost)

Got to leave now. I'll continue the review on Monday. :-)