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

Revision history for this message
Gerry Boland (gerboland) wrote :

> > 3 nested namespaces, can't we reduce that?
>
> It's not my personal preference either, but more important is to keep
> consistency between the apis. As lp:unity-api has been defined with this
> namespace structure, we need to adopt it. Note that you don't need to use
> them in the actual implementation. Just "using namespace
> unity::shell:application" once in your header should be enough to not have
> to care about it all any more. Check out the launcher implementation in
> lp:unity8 plugins/Unity/Launcher/.

Ok, I'll accept for consistency with what's already in unity-api. But we should have a chat about it later on.

+ * @bried The currently focused application.
Typo: "brief"

startProcess & stopProcess - sorry I just realized that the names are next exactly correct, as an application may launch several processes (e.g. webbrowser-app spawns a QWebProcess for each tab). Perhaps rename to just "start" and "stop", or "startApplication" and "stopApplication"

Only other issue is in your tests, that "Ubuntu.Application" is not a clear name for the plugin. I'm using "Unity.ApplicationManager" in unity-mir which is hopefully more descriptive.

Otherwise, it's excellent!

« Back to merge proposal