davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  Concept is good.

INLINE COMMENTS

> colorscope.cpp:127
>      }
> +    m_lastGroup = m_group;
>      return m_group;

It's weird to be caching in a public getter.

It opens us up for problems

If some client code (for whatever reason) did 
connect(scope, inheritChanged, []() {scope->colorGroup());

then our signals in checkColorGroupChanged won't get emitted.

Can we move this member var into checkColorGroupChanged? Means we can get rid 
of the mutable too

REPOSITORY
  R242 Plasma Framework (Library)

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

To: apol, #plasma, mart, davidedmundson
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

Reply via email to