[Merge] lp:~thomas-voss/trust-store/add_convenience_function_for_processing_incoming_requests into lp:trust-store

[Marcus Tomlinson]

Review: Needs Fixing

Ok, so there's a fair amount of code here, I hope I haven't missed anything major.
Here are a few observations:

35	+               qt5-default,
36	+               qtbase5-dev,

AFAIK qtbase5-dev is included in qt5-default

202	+     * @param app_id The application Id of the requesting application.

OCD I know, but the everywhere else you've used "id" instead of "Id" (Should be "application id").

320	+ * issueing a prompt request via the given agent to the user. On return, the given trust-store is up-to-date.

Should be: "issuing a prompt..." 

334	+ *         core::trust::RequestParameters params
335	+ *         {
336	+ *             trust.agent,
337	+ *             trust.store,
338	+ *             app_id,
339	+ *             default_feature,
340	+ *             "Application " + app_id + " wants to access the example service."
341	+ *         };

You're missing "app_pid" (3rd param) between "trust.store" and "app_id".

362	+ *         std::shared_ptr<Store> store

Should be "std::shared_ptr<core::trust::Store> store"
(as it is for "core::trust::Agent" before this)

389	+ * resources. For that, we severly limit an application's access to the system and

Should be "we severely limit..."

716	+    static auto child_setup = []() {};

() not needed. Could just be: "static auto child_setup = []{};"
Just a personal style thing I guess.

854	+std::shared_ptr<core::trust::Agent> mir::create_agent_for_mir_connection(MirConnection* connection)

The implementation of this method (although impressively nested) is really hard to read. Just saying...

990	+// Abstracts common functionality required for runninging external helpers.

"runninging" ;)

1154	+inline QString appDirectory() {
1155	+    if (isRunningInstalled()) {
1156	+        return QString("@CMAKE_INSTALL_PREFIX@/@APP_DIR@/");

Is this not supposed to return "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_BINDIR@" as is validated in isRunningInstalled()? (I'm probably just confused here)

1174	+ * This file is part of dialer-app.

Really? You have 3 references to "diaper-app" in this header comment.

1255	+    if (!parser.isSet(core::trust::mir::cli::option_title))
1256	+        abort(); // We have to call abort here to make sure that we get signalled.

This app (trust-prompt) just aborts if the arguments are incorrect. Could you add a -h / --help handler to display the intended usage.

Also, when I run trust-prompt, the width of the window that appears seems to be too short (the left side of the "Deny" button is cut off a bit).

1879	+    auto a_spinning_process = []()

Again, could just be: "auto a_spinning_process = []"
-- 
https://code.launchpad.net/~thomas-voss/trust-store/add_convenience_function_for_processing_incoming_requests/+merge/226696
Your team Ubuntu Phablet Team is subscribed to branch lp:trust-store.