[Bug 43599] Add high-level API for Conn.I.Addressing interface

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Dec 19 14:54:43 CET 2011


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

--- Comment #6 from Andre Moreira Magalhaes <andrunko at gmail.com> 2011-12-19 05:54:43 PST ---
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Mmm, I wonder
> > > 
> > > > cmake: Fix FindDBusGLib.cmake to properly search for dbus-glib-1 instead of dbus-glib.
> > > 
> > > How did this work before? Didn't we use it at all or something? Or is there now
> > > some chance that an installation where it worked before, is now broken?cmake: 
> > 
> > It was working before cause most distros install dbus-glib in /usr/include, so
> > even with pkg_check_modules failing it was getting the headers from
> > /usr/include/dbus-1.0/*
> > But now it properly uses pkg-config for tips on where to find the headers, as
> > it should.
It is because we were searching for dbus-1.0/dbus/dbus-glib.h

find_path(DBUS_GLIB_INCLUDE_DIR
          NAMES dbus-1.0/dbus/dbus-glib.h
...

So /usr/include/dbus-1.0/dbus/dbus-glib.h was being found.

> > > @@ -565,10 +570,22 @@ void
> > > PendingContacts::onAddressingGetContactsFinished(PendingOperation *operatio
> > > 
> > > This code extracting the attributes and the referenced handles etc also looks
> > > awfully familiar. Could you please refactor it so that it shares the code with
> > > the codepath I originally wrote that in, rather than copypasting?
> > The code here is similar but has some few differences, so no change here.
> > 
> 
> ++ for now, until PendingContacts is otherwise refactored some time in the
> future. Could you add a fixme to do that please? I believe we can then
> rearrange the code so that actually the total number of lines goes up slightly,
> but the complexity goes down severely and it's much easier to follow and make
> changes in isolation, including sharing the parts of code like this which can
> be shared.
FIXME added to pending-contacts.cpp, to refactor it in the future

> > > + *
> > > + * See addressesable addresses specific methods' documentation for more
> > > details.
> > > + *
> > > + * \sa addressableVCardAddresses(), addressableUris()
> > > 
> > > The "See..." line is rather vague, but also redundant with the semantical \sa
> > > line, so I'd just remove it. The actual \sa's in this commit look very good in
> > > general though.
> > Yes, I agree the "See ..." line is vague, but it's used everywhere else, even
> > in all other contact features docs. So for keeping consistency I didn't change
> > this.
> > 
> 
> Well. Frankly put, you shouldn't duplicate crap just to be consistently crappy.
> We've tried to weed zero-information "see also" things out as much as possible,
> e.g. making the references in TextChannel reasonable as part of the last big
> doc cleanup you admirably carried out.
> 
> The important point here is that *there are no more details regarding the
> feature in the accessor documentation* so you can't "see" it to get more
> details on the feature. For some other features, there is, though yeah, some of
> them are just as wrong as this one which has now been written. But let's not
> worry about cleaning up them now, though still, let's not add MORE misleading
> references.
> 
> Anyway, do whatever you please with the other accessors, but let's not make
> these newly written docs misleading just for consistency. Merge after proto
> addressing branch is merged (just now last pass -reviewed it), and cleaning
> this thing up one way or another, please.
Ok, done, also removed the "see also" from the other Contact feature docs.

Thanks for the review, will merge when merging proto-addressing.

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



More information about the telepathy-bugs mailing list