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