Merge lp://qastaging/~compiz-team/compiz/compiz.gtk-window-decorator-gsettings into lp://qastaging/compiz/0.9.8

Coverage: http://people.canonical.com/~smspillaz/coveragereports/compiz.gtk-window-decorator-gsettings/

Can't build, for some reason, on precise:

[ 19%] Recompiling GSettings schemas locally
/bin/sh: 1: --targetdir=/home/dan/bzr/compiz/tmp.gwd/build/generated/glib-2.0/schemas/: not found
make[2]: *** [generated/glib-2.0/schemas/gschemas.compiled] Error 127
make[1]: *** [gtk/window-decorator/CMakeFiles/compiz_gsettings_compile_local.dir/all] Error 2

And again, please make sure upper-case is only used for macros. Everything else should have some lower case.

Thanks I'll look into that.

Sent from Samsung Mobile

 Daniel van Vugt <email address hidden> wrote:

null

Hi,

So it looks like we both fixed the build issue at the same time. As for the constants - I started to change all of the uppercase ones to lowercase but decided only to do it for constants which should have been function-local anyways.

Generally speaking I don't like working with global data unless its constant - macros have been historically upper case for this reason, and I think that the global constants fit into this category as well. The fact that they are uppercase immediately alerts the maintainer that they are working with program-local data.

I think it also aides in the case of confusing global data for local data. For example:

const gchar *org_foo = "org.foo";

...

GSettings *
instantiate_some_schema_foo ()
{
    GSettings *foo_schema = g_settings_new (org_foo);
    return foo_schema;
}

as opposed to:

const gchar *ORG_FOO = "org.foo"

...

GSettings *
instantiate_some_schema_foo ()
{
    GSettings *foo_schema = g_settings_new (ORG_FOO);
    return foo_schema;
}

Its quite obvious that ORG_FOO has some special meaning in the second case just by looking at the function alone, wheras in the first case one might be left scratching their head as to where org_foo came from. In addition, there's also no worry about shadowing conflicts

const gchar *org_foo = "org.foo"

...

GSettings *
instantiate_some_schema_foo ()
{
    GSettings *org_foo = g_settings_new (org_foo);
    return org_foo;
}

If one used the latter, and stuck to the "global constant data is uppercase, everything else is lowercase" rule there'd be no room for naming conflicts.

That's why I haven't changed the constants that are global like ORG_COMPIZ_GWD etc.

I disagree because:

1. Finding where something is defined is no easier or harder if it is upper or lower case.

2. All C/C++ programmers are used to the convention of uppercase for macros only. So you should not break peoples' mental model. Although, very often you still need to know if MY_STRING_CONSTANT is addressable, I admit.

3. const gchar *ORG_FOO is a variable. I think you meant to make it a constant. Which should be either:
     const gchar * const ORG_FOO = "org.foo";
or
     const gchar ORG_FOO[] = "org_foo";

4. Your masking example should actually just be:
    return g_settings_new (org_foo);
which has no masking.

But enough pedantism. I won't block on this issue.

The following tests FAILED:
   6 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetUseTooltips/0 (Failed)
   7 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetDraggableBorderWidth/0 (Failed)
   8 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetAttachModalDialogs/0 (Failed)
   9 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetBlur/0 (Failed)
  10 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetButtonLayout/0 (Failed)
  11 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetOpacity/0 (Failed)
  12 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetMetacityTheme/0 (SEGFAULT)
  13 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetFont/0 (SEGFAULT)
  14 - GSettingsStorageUpdates/GWDSettingsTestStorageUpdates.TestSetTitlebarActions/0 (SEGFAULT)
Errors while running CTest

Please be sure to Resubmit new revisions, and not make changes while Status is "Needs review"