| Summary: | high-level interface for Conn.I.Balance | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Danielle Madeley <danielle> |
| Component: | tp-glib | Assignee: | Telepathy bugs list <telepathy-bugs> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | CC: | pochu27 |
| Version: | git master | Keywords: | patch |
| Hardware: | Other | ||
| OS: | All | ||
| URL: | http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=balance-feature-36334 | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Bug Depends on: | |||
| Bug Blocks: | 37358 | ||
We usually put the test CM in tests/lib and only the actual test in tests/dbus/. Any chance to convince you to wait that my refactoring in bug #31583 is merged and rebase your branch on top of it? It would make your code easier and would save me another conflict hell... I just merged my refactoring so I suggest you to rebase your branch on top of master and use the new feature API. I've rebased it here: http://cgit.collabora.com/git/user/pochu/telepathy-glib.git/log/?h=balance-feature-36334 A review would be very welcome - goto finally;
+ g_simple_async_result_set_from_error (result, in_error);
}
Immediately noticed you've removed the goto finally; here. You don't want to do this because @props will be NULL.
(In reply to comment #5) > - goto finally; > + g_simple_async_result_set_from_error (result, in_error); > } > > Immediately noticed you've removed the goto finally; here. You don't want to do > this because @props will be NULL. Right, amended (added a return; ). I wonder if we need the g_object_freeze_notify() / g_object_thaw_notify() around tp_connection_unpack_balance(), given that tp_connection_unpack_balance() calls them itself. Is that so notify::balance-uri is emitted together with the other notify signals? Is it a problem if they aren't? If so, we could avoid that by adding an optional const gchar *balance_uri parameter to tp_connection_unpack_balance(). That would keep the code somewhat cleaner IMHO. I don't see a return. Did you forget to push? The freeze/thaw is so that all of the property notifications are emitted together. I don't know if this is needed, but it felt nice to have. Since some combination of the 3 properties can update together, binding to 3 properties, and thus getting 3 callbacks, seems suboptimal. Perhaps we also need a ::balance-changed(balance, scale, currency) signal. Oops, pushed now. - goto finally; + g_simple_async_result_set_from_error (result, in_error); + return; Never calls g_simple_async_result_complete()... goto finally; would be better. Who is going to add the signal, you or I? Finally I did it as I'd really like to get this in for the next release: http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=balance-feature-36334 (In reply to comment #10) > Finally I did it as I'd really like to get this in for the next release: > http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=balance-feature-36334 + * @balance_scale: the value of the #TpConnection:balance-currency property + * @balance_currency: the value of the #TpConnection:balance-scale property Backwards! Fixed and merged to master; will be in 0.15.1 |
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.
cassidy writes: > Can you please open a tp-glib bug asking for high-level API for Balance and > link it in the code? I try to keep track of places where we are using the raw > API.