[Bug 27727] account storage plugins can contain partial accounts

[bugzilla-daemon at freedesktop.org]

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

--- Comment #39 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
(In reply to comment #31)
> > 05/26] McpAccountStorage: have a default implementation for  every method
> > 
> > Based on a patch by Xavier Claessens.
> 
> I don't think we want default implementation for list() and get(), they are
> still needed for any useful storage, no?

Fair point. I can delete the default implementations for things that any useful
plugin must override, if you'd prefer.

I think that means:

* list
* owns (but I deleted that later in the branch anyway)
* get (but I deleted that later)
* get_attribute (must be able to get at least "manager" and "protocol")
* get_parameter (well I suppose in theory a CM might not need parameters but...
no)

(In reply to comment #32)
> (In reply to comment #14)
> > 10/26] mcd_account_delete: convert into  mcd_account_delete_async
> 
> migrate_delete_account_cb() looks weird, why don't you put directly
> mcd_account_delete_debug_cb as callback and free the MigrateCtx before the
> async call?

Because migrate_ctx_free() isn't 100% a free-function: it also releases a
"lock" that prevents MC from saying it's ready until all migrations have
completed.

> While we are adding async API, why omitting the GCancellable? I think you
> can just have one and put it on your GTask, it will do the right thing for
> you.

Hmm. I'm not sure what our rule should be on this: you can't *actually* cancel
deletion of an account, only the callback. Then again, g_dbus_connection_call()
takes a cancellable, and "send a D-Bus message" is pretty much the archetype of
things you can't cancel, so... yeah, OK, you're right.

(In reply to comment #33)
> Not sure to understand why MC would ever create a system bus, modules doing
> it behind our back?

GNetworkMonitor might use the system bus, and McdConnectivityMonitor certainly
does (to (try to) talk to systemd-logind).

> You can also just use g_assert_no_error() instead of
> more complex error handling, no?

Yeah, I suppose so, since it's just for the tests.

(In reply to comment #34)
> mcp_account_storage_delete_finish()'s doc should tell about the default
> implementation.

Sure.

> Do we recommend to override _finish() when _async is
> overriden, or can we state that if _async is implemented using GTask then
> the default _finish is good? I can guess your answer on this, but better to
> say it explicitly in the doc, no?

I think we should say you have to override neither or both: in some older
Telepathy APIs we had "the default uses a GSimpleAsyncResult", and now we're
regretting that because of GTask :-)

(In reply to comment #35)
> > 17/26] Make mcp_account_storage_create() mandatory
> 
> In mcd_storage_add_account_from_plugin() I would keep the comment my branch
> adds:
> /* This will fill our parameters/attributes tables */ because it's nowhere
> close to obvious.

OK, but only if I'm rebasing these changes in - later in the branch, that
strange setup disappears anyway.

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