+++ This bug was initially created as a clone of Bug #35896 +++ McdPluginAccountManager is a concrete class in MC. It implements the McpAccountManager GInterface, which is the API presented by MC to account storage plugins so they can call back into MC. The reason Mcp... is a GInterface is to present a rather limited API to plugins, so we're not prevented from making changes to the rest of MC's internals. McdPluginAccountManager also implements McdStorage, a GInterface used by the rest of MC's internals. This is, as far as I can tell, pointless: it's the only implementation of that interface, and every module that could access McdStorage can equally well access McdPluginAccountManager directly. I think we should flatten McdPluginAccountManager (class) and McdStorage (interface) into a single class. To reduce the diff churn, its name should be the name under which most of MC uses it, which is McdStorage.
Created attachment 66210 [details] [review] [1/5] McdMaster: fail to build if umask() is missing We ought to be able to rely on umask() for files created since 5.2.2, at least on Unix. --- See Bug #35896. Not really part of this bug, but it's easy to do and stops us having to chmod() things created by Bug #35896.
Created attachment 66211 [details] [review] [2/5] McdPluginAccountManager: fold interface methods into their implementations Nothing except McdPluginAccountManager implements McdStorage, and nothing ever will or should, so we can squash the two together.
Created attachment 66212 [details] [review] [3/5] McdStorage: remove _mcd_storage_store_connections I'm not sure why this was in McdStorage at all. It's a bit of a layering violation: AccountManager has a Storage, so Storage shouldn't call into AccountManager. We can fix the layering violation by adding a signal to McdAccount.
Created attachment 66213 [details] [review] [4/5] McdStorage, McdPluginAccountManager: squash into one class The Storage name stays, to make the diff more readable. The header is mcd-storage.h but the implementation is still plugin-account.c, for the same reason.
Created attachment 66214 [details] [review] [5/5] plugin-account.c: rename to mcd-storage.c --- Here's `git show -M`, which is easier to review: as you can see, it's just a rename and the necessary changes to src/Makefile.am to compensate for it. commit 4ba55e2ebbb080f37d731a750bf7c4880c560ea5 Author: Simon McVittie <simon.mcvittie@collabora.co.uk> Date: 2012-08-27 19:04:16 +0100 plugin-account.c: rename to mcd-storage.c diff --git a/src/Makefile.am b/src/Makefile.am index 0d09b7d..b9cd9df 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -159,6 +159,7 @@ libmcd_convenience_la_SOURCES = \ mcd-slacker.c \ mcd-slacker.h \ mcd-transport.c \ + mcd-storage.c \ mcd-storage.h \ plugin-dispatch-operation.c \ plugin-dispatch-operation.h \ @@ -166,7 +167,6 @@ libmcd_convenience_la_SOURCES = \ plugin-loader.h \ plugin-request.c \ plugin-request.h \ - plugin-account.c \ request.c \ request.h \ sp_timestamp.h \ diff --git a/src/plugin-account.c b/src/mcd-storage.c similarity index 100% rename from src/plugin-account.c rename to src/mcd-storage.c
This makes perfect sense, and the patches looks good to me. +1
Fixed in git for 5.13.1, thanks
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.