Mission Control contains a fork of servicetest with additional functionality. We should get Gabble and MC using the same version, then sync them from Gabble into all the other CMs.
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
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.