Code review comment for lp://staging/~ted/ubuntu-app-launch/app-object-signals

Revision history for this message
Michael Terry (mterry) wrote :

Ted asked me to look at this from a "is this API suitable for Unity8" perspective.

It seems fine to me. Mostly we are interested in delaying/stopping an app from starting if we're not in the right context (like non-Touch app on a phone). This API looks sufficient for that.

A couple notes though:

- Application::Instance does not provide access to arguments. But *maybe* the arguments might matter to approving a startup? I'm not sure what the use case for that would be. The manager would have to have knowledge of what arguments meant to a specific app. And the argument would have to trigger something that the user couldn't just do anyway in the app if it were started without the arg. So maybe there is no use case there... But something to consider. Can easily be added later if we do find we want it.

- There are a few "singal" typos in doc strings.

- Also in doc strings, you might want to clarify behavior around delayed replies. Like if the manager doesn't immediately get back to UAL, is there a timeout? What if another request comes in while the manager is considering what to do -- I'm assuming UAL gives that to the manager and lets it decide, rather than denying the second one quietly? This information would help me know how carefully I have to write the signal handlers.

« Back to merge proposal