[Bug 24656] Improve documentation

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24656


Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|telepathy-                  |andrunko at gmail.com
                   |bugs at lists.freedesktop.org  |
          QAContact|                            |telepathy-
                   |                            |bugs at lists.freedesktop.org




--- Comment #1 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2009-10-21 09:19:15 PST ---
Up to commit bfd6bb0bc6:

+ * \brief The AbstractClient class provides an object representing a
+ * <a href="http://telepathy.freedesktop.org">Telepathy</a> client.

Do we really need to hyperlink Telepathy? I think we can assume that Telepathy
users know where the website is from the README!

+ * The .client files must contain UTF-8 text with the same syntax as Desktop
+ * Entry files (although the allowed groups, keys and values differ). Every
+ * .client file must contain a group whose name is the name of this interface.
+ * See AbstractClientObserver::observerChannelFilter() for more details.

Perhaps this should just point into http://telepathy.freedesktop.org/spec/ ?

+ * The easiest way to create account objects is trough AccountManager. One can

through

+ * Another way to create an Account object is to just call the create method.

Could you expand on this a little to make it clear that ValidAccounts is the
list of stored accounts, and create() adds a new one? At the moment it reads as
though this is some sort of alternative API for the same thing.

+ * See avatar specific methods documentation for more details.

avatar-specific methods' documentation

+ *  <li>A fake group implementation when handle type is different from
+ *  HandleTypeContact</li>

when the handle type *is* HandleTypeContact, surely?

+ * undefined until the introspection process is completed; isReady() returns
true

until the introspection process is completed, i.e. isReady() returns true

(this text exists in several places)

+ * Channel object will transition to closed too.

I think we should document this in terms of invalidated - we want to encourage
people to see invalidated as "the" this-object-is-dead notification.

+ * As a result, approvers should use the first handler by default.

... by default, unless they have a reason to do otherwise.

You seem to be copying <tp:rationale> into these docstrings in a fairly
indiscriminate way. I don't think that's necessarily wise - the rationale is to
explain the spec, and not all of it needs to be in API docs (for that matter,
not everything from the spec needs to be in these docs - we can reference the
spec normatively). "More is better" is not necessarily true...

In particular, in PendingOperation *ChannelDispatchOperation::claim(), the
rationale seems very much out of place.

+ * A channel has closed before it could be claimed or handled. If this is
+ * emitted for the last remaining channel in a channel dispatch operation, it
+ * must immediately be followed by invalidated() with error
+ * %TELEPATHY_QT4_ERROR_OBJECT_REMOVED.

s/must/will/ to get this into the client's perspective, as usual

+ * This corresponds to the _NET_WM_USER_TIME property in EWMH.

(Unix developers: this corresponds ... in EWMH.)

Ideally we'd reference the Qt equivalent of gtk_window_present_with_time()
here, but we can't, because there doesn't seem to be one. Perhaps add a FIXME
comment pointing to a bug?

+ * Cancel the channel request. The precise effect depends on the current
+ * progress of the request.

I don't think the detailed explanation is useful to client developers; just
link to the description of Cancel() in http://telepathy.freedesktop.org/spec/
for the gory details. Only the last couple of paragraphs need to be kept:

+ * If failed() is emitted in response to this method, the error SHOULD be
+ * %TELEPATHY_ERROR_CANCELLED.
+ *
+ * If the channel has already been dispatched to a handler, then it's too late
+ * to call this method, and the channel request will no longer exist.

and "SHOULD" should be replaced with "will".

+ * Proceed with the channel request.

Does this method even need to be documented? If it's public, I suppose we have
to; if it's protected, just keep the first paragraph and point to the spec for
the rest.

+ * The easiest way to create connection objects is trough Account. One can

through

+ * Another way to create a Connection object is to just call the create
method.
+ * For example:
+ *
+ * \code ConnectionPtr am = Connection::create(busName, objectPath); \endcode

I think this clouds the issue of which one to use when. Perhaps "If you already
know the object path, you can just call ..."?

+ * property MAY be zero.

"may" please, this isn't a specification (unlike telepathy-spec, which uses
RFC-style MAY/SHOULD/MUST).


-- 
Configure bugmail: http://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.