D16875: [kded] Cleanup KScreenDaemon class

2018-11-14 Thread Roman Gilg 
This revision was automatically updated to reflect the committed changes.
Closed by commit R104:4a177884b052: [kded] Cleanup KScreenDaemon class 
(authored by romangg).

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16875?vs=45466=45469

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

AFFECTED FILES
  kded/daemon.cpp
  kded/daemon.h

To: romangg, #plasma, davidedmundson
Cc: broulik, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16875: [kded] Cleanup KScreenDaemon class

2018-11-14 Thread Roman Gilg 
romangg updated this revision to Diff 45466.
romangg added a comment.


  - Add back Q_SLOTS specifier to be sure

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16875?vs=45448=45466

BRANCH
  codeCleanupExport

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

AFFECTED FILES
  kded/daemon.cpp
  kded/daemon.h

To: romangg, #plasma, davidedmundson
Cc: broulik, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16875: [kded] Cleanup KScreenDaemon class

2018-11-14 Thread David Edmundson 
davidedmundson added inline comments.

INLINE COMMENTS

> romangg wrote in daemon.h:47
> Are you sure this is a problem? I just tested it manually with qdbusviewer 
> and the method was called accordingly.
> 
> Nowadays in Qt can't every function act as a slot, even without the Q_SLOTS 
> specifier?

If it's done with QDBusConnection::registerObject(this)  then it definitely 
needs runtime introspection.

If it's done with an adaptor pattern, then maybe not.

REPOSITORY
  R104 KScreen

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

To: romangg, #plasma, davidedmundson
Cc: broulik, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16875: [kded] Cleanup KScreenDaemon class

2018-11-14 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> romangg wrote in daemon.h:47
> Are you sure this is a problem? I just tested it manually with qdbusviewer 
> and the method was called accordingly.
> 
> Nowadays in Qt can't every function act as a slot, even without the Q_SLOTS 
> specifier?

For function ptr connect, yes, for runtime introspection, no

REPOSITORY
  R104 KScreen

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

To: romangg, #plasma, davidedmundson
Cc: broulik, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16875: [kded] Cleanup KScreenDaemon class

2018-11-14 Thread Roman Gilg 
romangg added inline comments.

INLINE COMMENTS

> davidedmundson wrote in daemon.h:47
> this has moved from being a slot.
> 
> If it is invoked from DBus as the comment suggests that will be a problem

Are you sure this is a problem? I just tested it manually with qdbusviewer and 
the method was called accordingly.

Nowadays in Qt can't every function act as a slot, even without the Q_SLOTS 
specifier?

REPOSITORY
  R104 KScreen

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

To: romangg, #plasma, davidedmundson
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16875: [kded] Cleanup KScreenDaemon class

2018-11-14 Thread David Edmundson 
davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> daemon.h:47
> +// DBus
> +void applyLayoutPreset(const QString );
> +

this has moved from being a slot.

If it is invoked from DBus as the comment suggests that will be a problem

REPOSITORY
  R104 KScreen

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

To: romangg, #plasma, davidedmundson
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16875: [kded] Cleanup KScreenDaemon class

2018-11-14 Thread Roman Gilg 
romangg created this revision.
romangg added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
romangg requested review of this revision.

REVISION SUMMARY
  Do not export class, privatize and unvirtualize members.
  Cleanup white space.

TEST PLAN
  Manually in Wayland session.

REPOSITORY
  R104 KScreen

BRANCH
  codeCleanupExport

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

AFFECTED FILES
  kded/daemon.cpp
  kded/daemon.h

To: romangg, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart