[Bug 36881] Wrong documentation for Tp::Account::connectionStatusChanged (and a bunch of other parts of the API)

[bugzilla-daemon at freedesktop.org]

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

--- Comment #2 from Olli Salli <ollisal at gmail.com> 2011-05-31 12:13:15 PDT ---
Andre updated his branch. Re-reviewing.

The relationship of factories and becomeReady looks spot on now in the Account
and Connection docs!

+/**
+ * \fn QString Channel::GroupMemberChangeDetails::reason() const
+ *
+ * Return the reason for the change, if known.
+ *
+ * \return The reason for the change, or an empty string if the reason is
unknown.

The reason is not a string, it's a ChannelGroupChangeReason.

- * if supported, which is indicated by ChannelGroupFlagMessageDepart and/or
- * ChannelGroupFlagMessageReject.
+ * if supported, which is indicated by #ChannelGroupFlagMessageDepart and/or
+ * #ChannelGroupFlagMessageReject.
  *

Good catches! /me should learn to actually generate and see what the docs are
like when I'm writing API docs...

  * Cached access to state of the group interface on the associated remote
  * object, if the interface is present. Almost all methods return undefined
  * values if the list returned by interfaces() doesn't include
- * #TELEPATHY_INTERFACE_CHANNEL_INTERFACE_GROUP or if the object is not ready.
+ * #TP_QT4_IFACE_CHANNEL_INTERFACE_GROUP or if the object is not ready.

The accessors returning invalid stuff on non-Group channels hasn't actually
almost ever been true, due to the fact that we simulate the Group interface on
them.

  * corresponding groupMembersChanged() signal, as the local user being
- * removed usually causes the remote Channel to be closed.
+ * removed usually causes the remote channel to be closed.

I wonder what the "remote" word was, and is doing there.

 * Return whether the value returned by groupSelfContact() is guaranteed to
- * stay synchronized with what groupInterface()->GetSelfHandle() would
+ * stay synchronized with what group interface SelfHandle property would
  * return. Older services not providing group properties don't necessarily
  * emit the SelfHandleChanged signal either, so self contact changes can't be
  * reliably tracked.

The actual Channel.Group D-Bus interface properties are sufficiently obscure to
the user to this API that I don't think we should even mention them here.
Rather, it should say something like "groupSelfContact() is guaranteed to
accurately represent the local user even after nickname changes etc.". We
should also stress that this will always be true for a Group channel on any not
totally ancient service.


- * Return whether this channel implements the
- * org.freedesktop.Telepathy.Channel.Interface.MergeableConference interface.
+ * Return whether this channel implements the mergeable conference interface.

- * Return whether this channel implements the
- * org.freedesktop.Telepathy.Channel.Interface.Splittable interface.
+ * Return whether this channel implements the splittable interface.

Please either continue to mention the actual low-level interface name, or say
"supports conference merging" / "supports splitting", and refer to any actual
high-level operations we have on the subject. Just saying "the splittable
interface" is completely obscure.

+ * \return A pointer to the existing Client::ChannelInterface object for this
+ *         Account object.

Not an Account object!

Thanks for the clear organization of the commits, it made reviewing a breeze!

+ * Construct a info fields instance with the given details. The instance will
indicate it's valid.

"that it is valid", and we're not constructing from "details" but fields here.

+ *
+ * See avatar token specific methods' documentation for more details.

I don't think these kinds of comment lines are very helpful (Will agrees on bug
31769). Just what "more details" do the methods convey (and which ones are
they! I'm a beginning tp-qt4 programmer and can't tell!)?

If you want to have useful docs for the features, they must refer to the actual
accessors which they make to work.

Posting this now so you can work on these for the moment, will continue the
review in another comment.

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