[virt-tools-list] [PATCH virt-viewer v2 3/3] Write vm name to config file as comment

[Marc-André Lureau]

----- Original Message -----
> ----- Original Message -----
> 
> > From: "Fabiano Fidêncio" <fabiano at fidencio.org>
> > To: "Jonathon Jongsma" <jjongsma at redhat.com>
> > Cc: virt-tools-list at redhat.com
> > Sent: Tuesday, July 22, 2014 9:06:59 AM
> > Subject: Re: [virt-tools-list] [PATCH virt-viewer v2 3/3] Write vm name to
> > config file as comment
> 
> > On Mon, Jun 30, 2014 at 10:49 PM, Jonathon Jongsma < jjongsma at redhat.com >
> > wrote:
> 
> > > ---
> > > src/virt-viewer-app.c | 14 ++++++++++++++
> > > 1 file changed, 14 insertions(+)
> > 
> > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > > index 6bd49f4..158a638 100644
> > > --- a/src/virt-viewer-app.c
> > > +++ b/src/virt-viewer-app.c
> > > @@ -242,6 +242,20 @@ virt_viewer_app_save_config(VirtViewerApp *self)
> > > g_warning("failed to create config directory");
> > > g_free(dir);
> > 
> > > + if (priv->uuid && priv->guest_name) {
> > > + // if there's no comment for this uuid settings group, add a comment
> > > + // with the vm name so user can make sense of it later.
> > > + gchar* comment = g_key_file_get_comment(priv->config, priv->uuid, NULL,
> > > &error);
> > 
> > Coding style here, use: gchar *comment (...)
> 
> 
> I've been meaning to bring this up for some time actually. Currently, the
> spice coding style definitions (
> http://www.spice-space.org/docs/spice_style.pdf ) state that '*' should be
> attached to the type name, not the variable name (section 24.5). On the

The coding style doesn't work well for variables declaration:

char* a, *b; ?

So it's obvious to most people that it should be the variable that must have
the *. However, for function arguments, having the type* style is indeed nice.

Tbh, I don't think it's worth to standardize on this, or at least the Spice
style rule needs to changed.

> other hand, much of the code does not follow this guideline. It seems that
> in virt-viewer, the "wrong" way (according to the coding style document) is
> more common than the "right" way.  This is not a perfect measurement, but
> it's a decent approximation:
> 
> $ git grep "\w\* " -- src/ |wc -l
> 238
> $ git grep " \*\w" -- src/ |wc -l
> 1777

virt-viewer is not a Spice project only, and it was started before Spice support
was even added.

spice-gtk oth, is fully under Spice project, but I would prefer if it follows
more closely the GNOME/Gtk+ code style rather than Spice code style. However,
GNOME style being not stricly specified, spice-gtk is pretty loosily resembling it.

We could work on a "uncrustify" style, I use one for phodav and some other projects,
I know other GNOME devs are pretty happy with it too.
 
> So, should we just adopt the majority format and use "Type *varname" for all
> new code?

Imho, we should remove that rule from code style, or specify Type* for function
arguments and Type *var for variables.