[Bug 27669] Implement Location interface support

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Wed Apr 21 13:45:24 CEST 2010


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

--- Comment #4 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-04-21 04:45:24 PDT ---
(In reply to comment #1)
> Please separate this into two branches

Or three branches. That works too :-)

Basic codegen
=============

review+ for andrunko/location.

Other people's locations
========================

> > +    if (!mPriv->requestedFeatures.contains(FeatureLocation)) {
> 
> Is requestedFeatures really the right member there?

I still think this is wrong.

> > -QVariantMap Contact::location() const
> > +ContactLocation *Contact::location() const
> 
> This seems to return a pointer that's only valid as long as the Contact is. Is
> that appropriate? It should at least be documented.
> 
> It should probably either be a const pointer that's documented to have the same
> validity as the Contact, or a refcounted Tp::ContactLocationPtr?

I still think this is wrong, and I'd be inclined to define and use a
Tp::ContactLocationPtr (or you could turn ContactLocation into a value type
with a refcounted private struct, like QString, if you prefer).

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.



More information about the telepathy-bugs mailing list