Bug 69822

Created attachment 86579 [details] [review]
[Gabble master, 1/3] Add lots more constants, from Mission Control

I kept Gabble's naming where it conflicted, except for replacing
UNKNOWN_METHOD with DBUS_ERROR_UNKNOWN_METHOD which I think
is better.

Created attachment 86580 [details] [review]
[Gabble master, 2/3] servicetest: add infrastructure for faking D-Bus  services, from MC

I accidentally a small Python D-Bus binding.

Created attachment 86581 [details] [review]
[Gabble master, 3/3] servicetest: simplify sync_dbus() to the version from MC

Created attachment 86582 [details] [review]
[MC master, 1/3] Include config.h in every .c file (again)

This is potentially important if config.h includes things that affect
compilation globally, like ENABLE_DEBUG, "#define inline __inline",
or GLib, Telepathy, etc. versioned deprecation macros.

<config.h> rather than "config.h" as recommended in the Autoconf manual.

---

Not really related to this but we have enough bug reports floating about already.

Created attachment 86583 [details] [review]
[MC master, 2/3] configure.ac: use AC_DEFINE instead of appending to  CFLAGS

In build systems that put CFLAGS in an environment variable rather
than a configure command-line argument (e.g. RPM, I think?), what we
put in CFLAGS will potentially take precedence, and the external
build system's chosen CFLAGS won't necessarily be used. Meanwhile,
if the build system puts CFLAGS on the make command line, it will
take precedence: in particular, debug logging could be disabled, which
is bad for debuggability. If we move to AC_DEFINE, they'll interact
properly.

There is one remaining use of CFLAGS in configure.ac, for code
coverage. RPMs and other production-quality builds shouldn't have
coverage instrumentation, so this doesn't really conflict.

Also use AS_IF instead of if/fi, while I'm touching these lines anyway:
it's better Autoconf style.

Created attachment 86584 [details] [review]
[MC master, 3/3] Sync servicetest, constants with the version merged into  Gabble

---

There's quite a diffstat here because I preferred the naming from Gabble, on the basis that that's what every other CM syncs with.

I think we should probably treat MC as the master copy in future: it's the one with the most complicated requirements.

Comment on attachment 86579 [details] [review]
[Gabble master, 1/3] Add lots more constants, from Mission Control

Review of attachment 86579 [details] [review]:
-----------------------------------------------------------------

++

Comment on attachment 86580 [details] [review]
[Gabble master, 2/3] servicetest: add infrastructure for faking D-Bus  services, from MC

Review of attachment 86580 [details] [review]:
-----------------------------------------------------------------

++ assuming it's existing code from MC

Comment on attachment 86581 [details] [review]
[Gabble master, 3/3] servicetest: simplify sync_dbus() to the version from MC

Review of attachment 86581 [details] [review]:
-----------------------------------------------------------------

++

Comment on attachment 86582 [details] [review]
[MC master, 1/3] Include config.h in every .c file (again)

Review of attachment 86582 [details] [review]:
-----------------------------------------------------------------

++

I always thought that "config.h" was the proper one. :(

Comment on attachment 86583 [details] [review]
[MC master, 2/3] configure.ac: use AC_DEFINE instead of appending to  CFLAGS

Review of attachment 86583 [details] [review]:
-----------------------------------------------------------------

++

Comment on attachment 86584 [details] [review]
[MC master, 3/3] Sync servicetest, constants with the version merged into  Gabble

Review of attachment 86584 [details] [review]:
-----------------------------------------------------------------

++

I sometimes wonder if it wouldn't be easier to have a 'common' submodule shared accross components (like GStreamer does) instead of copying those files manually.

(In reply to comment #13)
> I sometimes wonder if it wouldn't be easier to have a 'common' submodule
> shared accross components (like GStreamer does) instead of copying those
> files manually.

Then we wouldn't have to copy files around \o/ but we'd have submodules /o\

Applying a patch to a submodule is annoying: you can't just "git format-patch" and use it in a distro, you have to sed all the filenames.

I'll send a mail to the Telepathy list at some point (later today?) with some thoughts about merging git repositories together, which will mitigate this.

Fixed in git for Gabble 0.19.0, MC 5.17.0