On Thu, 2012-02-16 at 10:50 +0100, Patrick Ohly wrote:
> On Thu, 2012-02-16 at 10:03 +0100, Krzesimir Nowak wrote:
> > 2012/2/16 Patrick Ohly <[email protected]>:
> > > My goal is to merge all of the D-Bus binding improvements into master as
> > > soon as possible. I've already rewritten some parts of the syncevolution
> > > binary's D-Bus calls to be blocking and it turned out much nicer than
> > > the older code. The PBAP backend would also benefit from proper blocking
> > > calls.
> > >
> > >> [1] 
> > >> https://meego.gitorious.org/~krnowak/meego-middleware/krnowaks-syncevolution/commits/cssr1
> > >
> > > Some quick comments about that branch: please write a bit more verbose
> > > commit messages. Just a single line is often not enough to understand
> > > what the commit is about. Looking at the code helps, but takes a lot
> > > more time. If the code is wrong, that kind of analysis doesn't help
> > > either.
> > >
> [...]
> > Ok, I will try to follow the guidelines in my commits.
> 
> Thanks, that'll make my life easier.

For example, why did you change this:

commit c7a6f28162b5af2d42b41393dc12a16583482312
Author: Krzesimir Nowak <[email protected]>
Date:   Mon Feb 6 16:48:47 2012 +0100

    GDBus GIO: don't expose GIO GDBus API in wrapper
    
    In this case - GDBusConnection. Replaced it with DBusConnectionPtr.

diff --git a/src/gdbusxx/gdbus-cxx-bridge.h
b/src/gdbusxx/gdbus-cxx-bridge.h
index 4d60da0..2464b0b 100644
--- a/src/gdbusxx/gdbus-cxx-bridge.h
+++ b/src/gdbusxx/gdbus-cxx-bridge.h
@@ -445,7 +445,7 @@ class DBusObject
         }
     }
 
-    GDBusConnection *getConnection() const { return m_conn.get(); }
+    DBusConnectionPtr getConnection() const { return m_conn; }
....

Regardless whether we return GDBusConnection or a DBusConnectionPtr, the
fact that there is a GDBusConnection is visible to the user of the API.

On the practical side, now with DBusConnectionPtr, a lot of code has to
make explicit get() calls, which I consider a drawback of that approach.

I stumbled over this because the patch itself is also incomplete. There
are other places where getConnection() still returns a plain pointer.
That in fact breaks compilation of the cssr1 branch in combination with
GDBus libdbus - see my latest push to that branch.

I'm currently leaning towards simply reverting the commit above. Before
I do that I'd like to understand the motivation for it better.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.


_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution

Reply via email to