Code review comment for lp:~mzanetti/unity-api/launcher-api-pinning

116 Q_INVOKABLE virtual void move(int oldIndex, int newIndex) = 0;

131 + Q_INVOKABLE virtual void pin(int index) = 0;

142 + Q_INVOKABLE virtual void remove(int index) = 0;

151 + Q_INVOKABLE virtual void triggerQuickListAction(int itemIndex, int quickListIndex) = 0;

I wonder if we should instead identify the items by their ids and not indexes? Maybe not for the move(), but for pin() / unpin() and trigger() I would.

pin() should also take the index at which to pin the item (when you drag an item onto the launcher from the dash). Not sure if remove should not be "unpin", exactly because it might not actually result in the removal of the item - it's the backend model that decides what happens.

=====

206 + * - RoleLabel: The text entry in the QuickList menu (QString).
207 + * - RoleIcon: The icon to be shown for this entry (QString).
208 + * - RoleCheckable: Wether the item is checkable or not (bool).
209 + * - RoleGroup: A group this item is in. Items in the same group are exclusive (int). -1 for no group.

220 + enum Roles {
221 + RoleLabel,
222 + RoleIcon
223 + };

Hmm does it?

Shouldn't that be "label", "icon", "checkable", "group", too, instead?

Wether → Whether.

"The icon" - should that be full URL or only a themed icon name?

"checkable" - can you reword?

=====

253 -add_subdirectory(copyright)
254 -add_subdirectory(whitespace)
255 +#add_subdirectory(copyright)
256 +#add_subdirectory(whitespace)

Oh no you don't! ;P

=====

Please put Authors: consistently below the copyright.

=====

Maybe put the SignalSpy at the top?