mpnordland
commented
2 years ago Another note, given that the documentation for the glib.MessageBus methods for connecting and introspecting use the (result, error) ordering and are documented correctly[1][2], I'm favoring correcting the documentation rather than changing the code.
1: https://python-dbus-next.readthedocs.io/en/latest/message-bus/glib-message-bus.html#dbus_next.glib.MessageBus.introspect 2: https://python-dbus-next.readthedocs.io/en/latest/message-bus/glib-message-bus.html#dbus_next.glib.MessageBus.connect
Documentation here: https://python-dbus-next.readthedocs.io/en/latest/high-level-client/glib-proxy-interface.html
It says:
But the code actually passes the result as the first argument and the exception as the second.
Method calls here: https://github.com/altdesktop/python-dbus-next/blob/master/dbus_next/glib/proxy_object.py#L126
Property gets here: https://github.com/altdesktop/python-dbus-next/blob/master/dbus_next/glib/proxy_object.py#L179
Property sets here: https://github.com/altdesktop/python-dbus-next/blob/master/dbus_next/glib/proxy_object.py#L228
It seems that the code is consistent about the order it uses, but the documentation doesn't match. It seems to me that if projects already use this code then the documentation should just be updated instead of possibly breaking existing applications. On the other hand, my guess is nobody uses these because I couldn't find an issue for it and this is pretty easy to bump into.
I checked the test suite and I found some tests for sync access to properties but not async access: https://github.com/altdesktop/python-dbus-next/blob/master/test/client/test_properties.py#L83
Probably should add some tests for the async access as well.
I'm happy to do a pull request for this, and either update the documentation or the code. I could also add some tests for the async stuff if that is wanted. Probably would need a little hand holding on the tests.
Thanks, Micah