D6963: Don't block starting notification service

2017-08-29 Thread David Edmundson
This revision was automatically updated to reflect the committed changes.
Closed by commit R289:1c97e1d9741f: Don't block starting notification service 
(authored by davidedmundson).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6963?vs=18912=18933

REVISION DETAIL
  https://phabricator.kde.org/D6963

AFFECTED FILES
  src/notifybypopup.cpp

To: davidedmundson, mart
Cc: alexeymin, marten, vandenoever, mck182, dvratil, #frameworks


D6963: Don't block starting notification service

2017-08-29 Thread Jos van den Oever
vandenoever added a comment.


  The patch looks clean, but I do not fully understand how is supposed to work. 
Perhaps an explanation of the different scenarios would help future readers of 
this code.

REPOSITORY
  R289 KNotifications

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6963

To: davidedmundson, mart
Cc: alexeymin, marten, vandenoever, mck182, dvratil, #frameworks


D6963: Don't block starting notification service

2017-08-29 Thread Marco Martin
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R289 KNotifications

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6963

To: davidedmundson, mart
Cc: alexeymin, marten, vandenoever, mck182, dvratil, #frameworks


D6963: Don't block starting notification service

2017-08-29 Thread David Edmundson
davidedmundson updated this revision to Diff 18912.
davidedmundson added a comment.


  Remove startFdo flag
  
  Keep windows away from DBus activation
  
  Better handle the service owner changing on a potentially 
  DBus activated notification handler

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6963?vs=17322=18912

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6963

AFFECTED FILES
  src/notifybypopup.cpp

To: davidedmundson
Cc: vandenoever, mck182, dvratil, #frameworks


D6963: Don't block starting notification service

2017-07-31 Thread Jos van den Oever
vandenoever added a comment.


  Removing this call is in line with the advice in the dbus spec.
  
  An improvement to the patch would be to remove `bool startfdo`.
  
  Q_WS_WIN seems to suggest that windows always needs to start the service 
manually. That case would break with this patch.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D6963

To: davidedmundson
Cc: vandenoever, mck182, dvratil, #frameworks


D6963: Don't block starting notification service

2017-07-31 Thread David Edmundson
davidedmundson added a comment.


  No, his one still blocks. This doesn't.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D6963

To: davidedmundson
Cc: mck182, dvratil, #frameworks


D6963: Don't block starting notification service

2017-07-28 Thread David Edmundson
davidedmundson created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  We don't need to manually start the DBus service. 
  It blocks the calling app, and dbusServiceExists means that we will
  always end up going the DBus route over a popup anyway, so it won't 
  do anything useful.
  
  The service (in the plasma case plasma-wait-for-name) will be started
  automatically when we actually send the notification.
  
  BUG: 382444

REPOSITORY
  R289 KNotifications

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6963

AFFECTED FILES
  src/notifybypopup.cpp

To: davidedmundson
Cc: #frameworks