D12936: Rewrite workspace KCM in QtQuick

Roman Gilg noreply at phabricator.kde.org
Thu May 17 09:20:27 UTC 2018


romangg added a comment.


  Please don't rename `workspaceoptions.h` and `workspaceoptions.cpp` to `kcm.h` and `kcm.cpp` in this patch to preserve the git history better. But imo you can remove Marco's copyright from these files, because you basically rewrote the files from scratch.

INLINE COMMENTS

> CMakeLists.txt:14
> +    KF5::I18n
> +    KF5::KIOCore
> +    KF5::ConfigWidgets

link not needed

> CMakeLists.txt:18
>  
> -target_link_libraries(kcm_workspaceoptions  KF5::KIOCore KF5::ConfigWidgets KF5::I18n)
> +    Qt5::QuickWidgets
> +)

link not needed

> kcm.cpp:108
> +    // Prevent from binding loop
> +    if( m_stateToolTip == state ) {
> +        return;

And a whitespace between `if` and `(`: https://community.kde.org/Policies/Kdelibs_Coding_Style#Whitespace

REPOSITORY
  R119 Plasma Desktop

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

To: furkantokac, romangg, ngraham, #plasma
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/4d2e6c30/attachment.html>


More information about the Plasma-devel mailing list