Description
Simon McVittie
2012-04-17 07:39:24 UTC
Created attachment 60172 [details] [review] [1/3] dbus_g_message_read_variants, dbus_g_message_write_variants: add and test These will let us send GVariant-based messages over libdbus connections, to facilitate porting from dbus-glib to GDBus. Created attachment 60173 [details] [review] [2/3] dbus_g_connection_call_async: add and test This is basically the same API as g_dbus_connection_call_async, but implemented in terms of libdbus. This means we can use a DBusGConnection in telepathy-glib to make method calls that look suspiciously like a GDBus call, reducing the size of the "flag day" commit we'll have to make when we switch from dbus-glib to GDBus. To support this, make GIO a public dependency. Created attachment 60174 [details] [review] [3/3] lcov.am: fix to work in out-of-tree builds Comment on attachment 60172 [details] [review] [1/3] dbus_g_message_read_variants, dbus_g_message_write_variants: add and test Review of attachment 60172 [details] [review]: ----------------------------------------------------------------- If you don't have DBusBasicValue you're also not going to have DBus8ByteStruct, so we'll need a private copy of that in dbus-gvariant.c for !HAVE_DBUSBASICVALUE. Other than that, nothing egregious pops out at me, and it passes the testcases. Comment on attachment 60173 [details] [review] [2/3] dbus_g_connection_call_async: add and test Review of attachment 60173 [details] [review]: ----------------------------------------------------------------- Perhaps I'm missing something huge, but where in dbus_g_connection_call_async() are the GVariant parameters set on the outgoing message? Return parameters are processed correctly, but it doesn't look like the outgoing message will have any arguments at all? Comment on attachment 60174 [details] [review] [3/3] lcov.am: fix to work in out-of-tree builds Review of attachment 60174 [details] [review]: ----------------------------------------------------------------- not really qualified to review this, but looks sane I'll return to this one day... but this is not that day. (In reply to comment #4) > If you don't have DBusBasicValue you're also not going to have > DBus8ByteStruct, so we'll need a private copy of that in dbus-gvariant.c for > !HAVE_DBUSBASICVALUE. True. At this point I think we could also just require dbus 1.6 - if you don't have that then you're just not keeping up. (In reply to comment #5) > Perhaps I'm missing something huge, but where in > dbus_g_connection_call_async() are the GVariant parameters set on the > outgoing message? Er... nowhere, but the test doesn't exercise parameters either, so I didn't catch this error. This needs fixing when I have time to resume dragging telepathy-glib into the future (or at least the present). Created attachment 87939 [details] [review] [01/11] dbus_g_message_read_variants, dbus_g_message_write_variants: add and test These will let us send GVariant-based messages over libdbus connections, to facilitate porting from dbus-glib to GDBus. --- Revised to actually handle (and test) parameters, and add documentation. Created attachment 87940 [details] [review] [02/11] dbus_g_connection_call_async: add and test This is basically the same API as g_dbus_connection_call_async, but implemented in terms of libdbus. This means we can use a DBusGConnection in telepathy-glib to make method calls that look suspiciously like a GDBus call, reducing the size of the "flag day" commit we'll have to make when we switch from dbus-glib to GDBus. --- Er, no, it's this one that was revised to handle parameters and add docs; the previous one was amended to not use DBus8ByteStruct (which it didn't actually need, because "char bytes[8]" was enough to force the size). You get the general idea, though. Created attachment 87941 [details] [review] [03/11] lcov.am: fix to work in out-of-tree builds --- The same as before. Created attachment 87942 [details] [review] [04/11] define MY_OBJECT_IFACE for tests to use Created attachment 87943 [details] [review] [05/11] Factor out a common test_assert_no_error macro Created attachment 87944 [details] [review] [06/11] test_run_until_round_trip: add Created attachment 87945 [details] [review] [07/11] Fix building other projects against an uninstalled dbus-glib The magic variables used in the -uninstalled.pc.in don't seem to be having the desired effect, and in any case, the standard Autoconf substitutions do what we want. Created attachment 87946 [details] [review] [08/11] Add GVariant-based signal subscriptions Created attachment 87947 [details] [review] [09/11] Add a test for DBusGProxy --- ... specifically, the new GVariant-based signal listening (yeah, this patch could have been named better). Created attachment 87948 [details] [review] [10/11] Add GVariant-based async method replies Created attachment 87949 [details] [review] [11/11] Add non-GError-based async method errors I'm abandoning this. Most dbus-glib projects have moved to GDBus anyway, the major exception being telepathy-glib which has a different migration strategy (although that migration has stalled). |
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.