Code review comment for lp://qastaging/~3v1n0/unity/lim

Revision history for this message
Marco Trevisan (TreviƱo) (3v1n0) wrote :

> 1) Please rename either the 'panel_service_show_entry' or the
> 'panel_service_actually_show_entry' functions. If 'panel_service_show_entry'
> does not show the entry then the function needs to be renamed.

The fact is that panel_service_show_entry is the public function used when the related method is called, while the panel_service_actually_show_entry is only a static function that I renamed to make common things.
I'll see if I can find a better name, but it makes sense to me anyway.

> 2) Another naming thing: PanelIndicatorEntryView::IsVisible is misleading,s
> ince it doesn't check whether the PanelIndivatorEntryView is visible or not,
> it check's whether the remote object is visible or not. Some alternative names
> might be:
>
> IsProxyVisible
> IsRemoteVisible

No, I don't want to do that, because the View is actually a kind of wrapper of the remote entry, when the remote entry is not visible, neither the view will be and back. Also all the current methods of that class refers to the view, even if they're currently using the values of the remote value. The fact that we have a remote value is a private thing, and we shouldn't care about that.

For the record, I've put this on WIP again because we need to include a new parameter to the EntryActivate signal to include the geometry of that entry.

« Back to merge proposal