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

Vlad Zahorodnii <[email protected]>
Newsgroups gmane.comp.kde.devel.kwin
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.