[Bug 69814] stop using the account id as a self id

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Sep 27 04:43:33 PDT 2013


https://bugs.freedesktop.org/show_bug.cgi?id=69814

--- Comment #9 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> ---
(In reply to comment #7)
> Possible refinement (non-review-blocker): if you used
> tp_tests_proxy_run_until_prepared(), you wouldn't have to pass in a main
> loop or have a callback, so your logic would be more linear (which is pretty
> useful for regression tests, which want to be as clear as possible).

changed.

(In reply to comment #8)
> ::: telepathy-logger/log-manager.c
> @@ +873,5 @@
> > +typedef struct
> > +{
> > +  GSimpleAsyncResult *result;
> > +  GSimpleAsyncThreadFunc func;
> > +} AsyncOpData;
> 
> If you used GTask you might not need this, but I think that's a job for
> post-1.0.

Agreed, post 1.0.

> @@ +923,5 @@
> > +  if (account != NULL)
> > +    {
> > +      GQuark features[] = { TP_ACCOUNT_FEATURE_CORE, 0 };
> > +
> > +      /* Most API rely on TpAccount being prepared so make sure it is */
> 
> I think it's worth being more specific, something like this:
> 
>        /* Most APIs rely on TpAccount being prepared, so make sure
>         * it is. telepathy-glib is not thread-safe, so we must do
>         * this in the main thread, before starting the actual
>         * operation in the other thread. */

changed.

> You can just use features = NULL I think? (No harm in being explicit,
> though.)

Yeah I prefer to be explicit.

> ::: tests/dbus/test-log-manager.c
> @@ +402,5 @@
> > +  account_path = g_value_get_string (
> > +      (const GValue *) g_hash_table_lookup (params, "account-path"));
> > +
> > +  account = tp_simple_client_factory_ensure_account (fixture->factory,
> > +      account_path, NULL, NULL);
> 
> If you intend the account to be unprepared here, please assert that it is
> unprepared.
> 
> (If it might be prepared already, you might have to create a new factory per
> test run in order to ensure duplication...)

Oh I used to have this assert actually but I guess I accidentally removed it.
Re-added.

Thanks for the review.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.


More information about the telepathy-bugs mailing list