Code review comment for lp://staging/~3v1n0/unity/launcher-draggable-icons

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

> 1959 - uscreen->resuming.connect([&]() -> void {
> 1960 - for (auto launcher : launchers)
> 1961 - launcher->QueueDraw();
> 1962 - });
>
> Why have you removed this code?

This code was added in order to workaround a nvidia redraw issue that we had on resuming, but since now we properly fixed the issue, we can safely remove this hack (tested here on one machine that was affected).

> In volume VolumeLauncherIcon you are calling parent_->UnStick(); before
> TryToUnblacklist. I think you should call it when you receive the changed
> signal from device_settings.

Why on changed signal?

> + void Stick(bool save = true);
>
> You should not use default values for virtual function (consider NVI to fix
> this "issue").

Yeah, this is not the best thing ever... It was introduced a lot time ago, probably it's better to split it into two methods (Stick and StickAndSave)

> 200 +#ifndef TEST_MOCK_DEVICES_H
> 4201 +#define TEST_MOCK_DEVICES_H
>
> Why do you need this guards?

Well, to avoid the redefinition of these objects. These won't probably be ever broken, but they won't surely create any issue.

« Back to merge proposal