Code review comment for lp:~paulliu/unity8/logout

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

Code looks ok.

Just some minor things that could be improved, but nothing blocker:

291 + onLogoutReady: {
292 + // close all the apps.
293 + while (true) {
294 + var app = ApplicationManager.get(0);
295 + if (app === null) {
296 + break;
297 + }
298 + ApplicationManager.stopApplication(app.appId);
299 + }

To keep things more organized, you could put that code in a helper function, like this:

"""
Connections {
   function closeAllApps() {
      ...
   }
   onLogoutReady: {
      closeAllApps();
      Qt.quit();
   }
}
"""

As a rule of thumb, when a block of code requires a comment explaining what it does, it usually means that it's doing a well defined task that could therefore be put into a separate function.

---------------------------------

218 + qmlRegisterSingletonType<DBusUnitySessionService>(uri, 0, 1, "DBusUnitySessionService", dbusunitysessionservice_provider);

That line is too long. Please break it.

review: Approve

« Back to merge proposal