Code review comment for lp:~canonical-dx-team/unity/unity.fix-685830

Revision history for this message
Mirco Müller (macslow) wrote :

> - It should be _panel_style not _panelStyle, same goes for anything else like
> that

Changed.
>
> - GetDefault(), if _panelStyle == NULL, then create a new one and return it,
> but if one is already around: return _panelStyle->Reference(); On the other
> end, the objects should grab panel style object on construction and then
> ->UnReference it on destruction.

But why should reference-counting be used, when PanelStyle is meant to be a singleton. I think these two concepts are mutual exclusive imo.

> - To be more C++, you should probably have:
>
> nux::Color& GetBackgroundTop ()

Initially you asked me to use ANSI-C-ish call-by-reference for this, but I've changed it now to the C++-reference return-type.

> - Probably can rid of printfs?

Yeah, was just for debugging.

> - I think the PanelIndicatorObjectEntryView bits still need to be hooked up
>
> - Missing a "changed" signal so we can react to theme changes?

Doing that atm.

« Back to merge proposal