D24862: Port one of session management connections state to a custom API

Vlad Zahorodnii noreply at phabricator.kde.org
Fri Oct 25 22:12:02 BST 2019


zzag accepted this revision.
zzag added a comment.
This revision is now accepted and ready to land.


  Some remaining nitpicks.

INLINE COMMENTS

> org.kde.KWin.Session.xml:5-8
> +      <method name="setState">
> +            <!-- set state. One of: normal, saving, quitting -->
> +            <arg name="state" type="s" direction="in"/>
> +    </method>

Indentation is off.

> sm.cpp:362
> +    new SessionAdaptor(this);
> +    QDBusConnection::sessionBus().registerObject("/Session", this);
>  }

Use QStringLiteral please.

> workspace.h:247
>  
> +    SessionManager* sessionManager() const;
> +

Style: Put whitespace before `*`.

> zzag wrote in workspace.h:743
> Style: We still follow the Frameworks coding style, which says that we have to put whitespace before `*`.
> 
> One could argue that return types have special meaning, but a coding review is not a place to enforce personal preferences that are uncommon or haven't been agreed upon by other developers. Such matter must be raised to the discussion in other places, e.g. a task or a mailing list. Also, no such a tool exists that would format code with this style.

This inline comment is not addressed.

REPOSITORY
  R108 KWin

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

To: davidedmundson, #kwin, zzag
Cc: romangg, zzag, kwin, LeGast00n, The-Feren-OS-Dev, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20191025/3c75b3cf/attachment-0001.html>


More information about the kwin mailing list