[virt-tools-list] [PATCH virt-viewer 2/3] app: Compute monitor mapping only in fullscreen

Fabiano Fidêncio fabiano at fidencio.org
Mon Feb 15 07:29:44 UTC 2016


Pavel,

On Mon, Feb 15, 2016 at 8:18 AM, Pavel Grunt <pgrunt at redhat.com> wrote:
> ---
>  src/virt-viewer-app.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index d4ce06a..19cc0a0 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -481,6 +481,10 @@ void virt_viewer_app_set_uuid_string(VirtViewerApp *self, const gchar *uuid_stri
>
>      g_free(self->priv->uuid);
>      self->priv->uuid = g_strdup(uuid_string);
> +
> +    if (!virt_viewer_app_get_fullscreen(self)) /* no need to get mapping */
> +        return;
> +

A few lines below we have comments in the format:
 // comment
 // foobar

Let's try to keep it standardized.

>      mapping = virt_viewer_app_get_monitor_mapping_for_section(self, uuid_string);
>      if (!mapping) {
>          g_debug("No guest-specific fullscreen config, using fallback");
> --
> 2.5.0
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list

On one hand I agree with the patch, but I'm still not sure if here is
the right place.
I do believe this set_uuid_string() is doing too much IMO. What do you think?

Anyways, patch is good.

Reviewed-by: Fabiano Fidêncio <fidencio at redhat.com>

-- 
Fabiano Fidêncio




More information about the virt-tools-list mailing list