Code review comment for lp:~abreu-alexandre/unity-webapps-qml/application-api

Revision history for this message
Alberto Mardegan (mardy) wrote :

I tested it and it's mostly working. A few comments:

getPlatformInfos: I think it's better to call it "getPlatformInfo" without the final "s". Anyway, this just returns the name of the QPA plugin, so I'd rather rename the method to "getPlatformName" or keep the current name and make it return a dictionary, currently having only one field "name" which contains the platform name. But I'd take the second approach only if you really plan to add more info there in the future.

The example expects the onActivated() and onDeactivated() signal to carry a parameter, but they don't.

All the methods are asynchronous and take a callback; even the applicationName() function is asynchronous, even if the result is immediately available. I'm not familiar with HTML5 development so this might be perfectly fine, I just point this out because coming from the C++ world this sounds rather strange (and a bit inconvenient).

Naming: it's better to use the same convention for all the getters: either start their name always with a "get", or never. Most of the methods you added start with "get", but "applicationName" doesn't.

getInputMethod(): it's not very clear what this function does; maybe rename it to "getInputMethodName"?

nameFromScreenOrientation(): you are only use two possible values, "Landscape" and "Portrait", while Qt supports 4. I guess that most apps don't care about distinguishing between the extra modes, but maybe some do (to be sure that the mic is on a specific side, for instance)?

« Back to merge proposal