Re: gdbus branch review

[David Zeuthen <zeuthen gmail com>]

Hey,

On Mon, Apr 23, 2012 at 9:39 AM, Alexander Larsson <alexl redhat com> wrote:
> I took a look at the gdbus branch. I didn't really try the code, as I
> assume its been tested. I did notice some issues in the code (below).
> Also, I think once these are fixed we should rebase the branch and merge
> the changes so that its a clean set of changes that don't e.g. add stuff
> and then remove it, or unnecessary split up changes because you found a
> bug later.
>
> ------------------------------------------------------------------------
>
> * monitor/proxy/dbus-interfaces.xml:
>
> This uses CamelCase for the arg names, this looks weird in C code

Just looking briefly over

 http://git.gnome.org/browse/gvfs/tree/common/dbus-interfaces.xml?h=gdbus-core
 http://git.gnome.org/browse/gvfs/tree/monitor/proxy/dbus-interfaces.xml?h=gdbus-core

I see this too and there are some inconsistencies too, for example
DriveStop() vs. lookupMount().

GNOME code generally uses CamelCase (not initialLowerCase) for
methods, signals and properties and lower_case for parameter names. At
least that's what gdbus-codegen(1) is optimized insofar that it
transforms names for use in C structs and C functions.

Of course, if you change lookupMount() to LookupMount() you
effectively change (break) the ABI and maybe you don't want that
(changing parameter names is not a break though). But then again maybe
you do, I mean, IIRC we never promised ABI stability on any of the
internals of GVfs. Then again, it's nice to not break ABI if you can
avoid it.

    David