On Thu, Mar 8, 2012 at 6:23 PM, Patrick Ohly <[email protected]> wrote:
> On Thu, 2012-03-08 at 17:18 +0100, Chris Kühl wrote:
>> On Thu, Mar 8, 2012 at 4:43 PM, Patrick Ohly <[email protected]> wrote:
>> > On Wed, 2012-03-07 at 16:54 +0100, Chris Kühl wrote:
>> >> Now that you've merged for-master/new-master into master we are
>> >> rebasing onto master.
>> >
>> > How is that going?
>> >
>>
>> It's been rebased and we've squashed the changes into about a dozen
>> commits. The tests seem to give us the same results as before the
>> rebase. See the concurrent-sync-sessions-rebased-pre-cleanup branch
>> for the rebased and squashed changes. I'm putting the final touches on
>> the cleanup know.
>
> 6c9a05a9db72f001d9834d2d24ac589f48fc5798
>
> dbus-server: Run sync sessions in separate processes
>
> ...
>
> Sessions are separated into SessionResource and Session classes. A
> SessionResource instance resides in the server process and serves as a
> proxy to the Session instance which is in the child process.
>
> This naming seems rather arbitrary to me. Why call it "Resource" and not
> something like "Stub" or "Proxy"?
>
Yeah, I'm not 100% happy with the naming either. They are subclasses
of Resource so it was the obvious choice. Renaming is not a problem
but I'd rather get finished with the more substantive changes needed
to complete this before doing that.
> Or, perhaps even better, don't rename it at all on the server side. Then
> a whole range of diffs goes away:
>
It's just that the Session is actually not in the Server anymore so it
seems a tad misleading to call it that in the Server.
> -------------------- src/dbus/server/auto-sync-manager.cpp
> --------------------
> index 53cdceb..a657f0a 100644
> @@ -17,10 +17,11 @@
> * 02110-1301 USA
> */
>
> #include "auto-sync-manager.h"
> -#include "session.h"
> +#include "session-resource.h"
> #include "server.h"
> +#include "dbus-callbacks.h"
> ...
> void AutoSyncManager::startTask()
> {
> // get the front task and run a sync
> // if there has been a session for the front task, do nothing
> - if(hasTask() && !m_session) {
> + if(hasTask() && !m_sessionResource) {
>
> But to be fair, I haven't checked whether that makes the diff larger
> elsewhere, for example in the actual implementation (session.h/cpp).
>
> Speaking of those, I'd like to see those moved to a separate directory
> eventually. There are different coding styles in server (strictly
> asynchronous) and in client (may block) and that should be made obvious
> by the location of the file.
>
>
You're speaking of keeping the files that are only used in the helper
in a subdirectory, right?
I'd rather do this towards the end of the process as well? Moving and
renaming files shouldn't be a problem as long as the other pieces are
fine.
Cheers,
Chris
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution