> On July 5, 2011, 7:41 p.m., David Edmundson wrote:
> > presence.cpp, line 45
> > <http://git.reviewboard.kde.org/r/101858/diff/1/?file=26121#file26121line45>
> >
> >     This isn't really ref counting.
> >     
> >     It will say how many of this type of object are in the system, not 
> > count how many pointers there are to this one instance. (actual ref 
> > counting)
> >     
> >     Anyway, I'd be very surprised if plasma worked this way. I'm pretty 
> > sure if an other plasmoid starts using it, it will increase the ref count 
> > of the dataengine and NOT call the constructor again.
> >     
> >     I would suggest looking where the parent class has it's ref counter 
> > increased.
> >     
> >     (arguably this will work to tell if the dataengine exists or not.. 
> > which is probably actually all that's needed in reality.)
> >     
> >     tl;dr pretty sure this won't work. Happy to be proved wrong if you have 
> > evidence otherwise.
> >     
> >
> 
> Martin Klapetek wrote:
>     You're right about the refCount, it is not an actual refCount as that 
> variable was initially used for refCount and then I changed it to static var, 
> but left the name like this. In the end it doesn't really matter, it works as 
> it should, really ;) It works in such way, that the dbus interface exposes a 
> method for returning this refCount var (whatever it really is) only if the 
> dataengine is created, when the dataengine is not in use (ie. deleted), the 
> dbus method does not exist, so in the end it serves its purpose. I can change 
> the variable to something like int isActive, but I think it's enough for a 
> temp workaround.

Change the name and I'll approve it.

Actually - why have this variable at all? 
If you get a dbus response it's active. If you don't - and get a dbus error, 
then it isn't. 

You may even simply be able to see if the dbus address exists and not need any 
methods in the address at all.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101858/#review4409
-----------------------------------------------------------


On July 5, 2011, 7:15 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101858/
> -----------------------------------------------------------
> 
> (Updated July 5, 2011, 7:15 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This simple patch adds a simple dbus adaptor to the dataengine, so it can be 
> checked if the plasmoid is in use (the dataengine is created only with the 
> plasmoid, when not needed, it is deleted). Next part will be checking from 
> contact list for this dbus entry/method. It is meant as a temp workaround for 
> bug 270675, once the upstream has proper features, we will remove it.
> 
> 
> This addresses bug 270675.
>     http://bugs.kde.org/show_bug.cgi?id=270675
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 69b839a 
>   presence.h 9b199c0 
>   presence.cpp 34fa5ea 
> 
> Diff: http://git.reviewboard.kde.org/r/101858/diff
> 
> 
> Testing
> -------
> 
> Added the plasmoid, dbus interface was created, removed the plasmoid, dbus 
> interface gets removed (though the path still stays, but the method is not 
> valid anymore).
> 
> 
> Thanks,
> 
> Martin
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to