D24862: Port one of session management connections state to a custom API
Vlad Zahorodnii
noreply at phabricator.kde.org
Thu Oct 24 13:34:52 BST 2019
zzag added inline comments.
INLINE COMMENTS
> sm.cpp:97
> cg.writeEntry("AllowsInteraction", sm.allowsInteraction());
> - sessionSaveStarted();
> if (gs_sessionManagerIsKSMServer) // save stacking order etc. before "save file?" etc. dialogs change it
> storeSession(config, SMSavePhase0);
Hmm, we no longer enter this path since gs_sessionManagerIsKSMServer is always false.
> sm.h:36
>
> +class SessionManager: public QObject
> +{
Style: Put whitespace between `SessionManager` and `:`.
> sm.h:41
> + SessionManager(QObject *parent);
> + ~SessionManager();
> +
Nitpick: In order to be consistent with other code, add `override` keyword.
> workspace.h:743
>
> -inline void Workspace::sessionSaveStarted()
> -{
> - session_saving = true;
> -}
> -
> -inline bool Workspace::sessionSaving() const
> +inline SessionManager* Workspace::sessionManager() const
> {
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.
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D24862
To: davidedmundson, #kwin
Cc: 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, romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20191024/1bedef71/attachment.html>
More information about the kwin
mailing list