Code review comment for lp:~dandrader/unity-api/mirSurface

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

On 16/07/15 19:14, Gerry Boland wrote:
> Review: Needs Fixing
>
> Thanks for all your changes so far, it's looking good. The consumesInput property will be fine, I understand it now.
>
>
> + enum Type {
> + UnknownType,
> ...
> + enum State {
> + UnknownState,
>
> Do we need the name of the enum appended to the name of each entry in the enum?
>
> We'll be using it like
> surface.type == Mir.UnknownType
> and repeating "type" feels a bit redundant to me.
>
> Is it the duplicate "Unknown" that inspired it?

That's convention for naming enum values. You have to mention somewhere
in the name value the enum it belongs to. It's like all those enums in
Qt namespace.

>
> Oh and do please bump the debian version.
>
> Don't we need to add some qml test to test/qmltest/unity/shell/application to check these objects actually contain these properties? Which means this interface has to be mocked. /me unsure

Can't really bump the debian version right now as it builds on top of a
yet unreleased MP (the app focus stuff). The debian/changelog would
conflict big time. So I will do it once the prerequisite gets released.

« Back to merge proposal