D12936: Rewrite workspace KCM in QtQuick

[Furkan Tokac]

furkantokac added a comment.


  In D12936#263969 <https://phabricator.kde.org/D12936#263969>, @ngraham wrote:
  
  > Please follow standard KDE style for braces in new code:
  >
  >   void myFunction() {
  >       int stuff = 1;
  >   }
  >
  
  
  According to Kdelibs Coding Style <https://community.kde.org/Policies/Kdelibs_Coding_Style>, functions and classes should be
  
    void fun()
    {
    }
  
  like this. Did I miss a point ?

INLINE COMMENTS

> ngraham wrote in kcm.cpp:51
> Why are these within braces?

To make the

  const KConfigGroup cg(config, QStringLiteral("PlasmaToolTips"));

this definition local.

> ngraham wrote in kcm.cpp:133
> How about `handleNeedsSave()` instead?

Makes more sense :) Done.

> ngraham wrote in ToolTip.qml:2
> If this code is used in multiple KCMs, we shouldn't duplicate it; we should upstream it so that it only needs to exist in one place at a time.

It can be reused. I'll delete it now (new patch is coming) then we can talk about it for the other patch.

REPOSITORY
  R119 Plasma Desktop

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

To: furkantokac, romangg, ngraham
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180517/85584e91/attachment.html>