D13931: [KCM] Add speaker placement test

David Rosca noreply at phabricator.kde.org
Thu Jul 12 15:28:25 BST 2018


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


  Looks much better now.

INLINE COMMENTS

> canberracontext.h:44
>  private:
> -    ca_context *m_context = nullptr;
> +    ca_context *m_canberra;
> +    int m_references;

`ca_context *m_canberra = nullptr;`

> canberracontext.h:45
> +    ca_context *m_canberra;
> +    int m_references;
> +

`int m_references = 0;`

> canberracontext.h:47
> +
> +    static CanberraContext* s_context;
> +

`static CanberraContext *s_context;`

And in other places too, `*` belongs to the variable name, not type.

> Advanced.qml:132
> +            property var role: sinkmodel.role("PulseObject");
> +            property var pulseObject: sinkmodel.data(index, role);
> +

Make it into one property, without `index` and `role`, either with pasting the code inline or moving it to separate function. Also it probably should be `readonly` property.

REPOSITORY
  R115 Plasma Audio Volume Applet

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

To: nicolasfella, drosca
Cc: ngraham, #vdg, 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/20180712/5014ac6e/attachment-0001.html>


More information about the Plasma-devel mailing list