[Bug 24656] Improve documentation

[bugzilla-daemon at freedesktop.org]

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





--- Comment #3 from Andre Moreira Magalhaes <andrunko at gmail.com>  2009-11-18 09:29:10 PST ---
(In reply to comment #1)
> 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!
TODO

> + * 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/ ?
TODO

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

> + * 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.
This actually does not create a new account. This is the Account::create
method, not the AccountManager::createAccount method, which actually attempts
to create a new account

> + * See avatar specific methods documentation for more details.
> 
> avatar-specific methods' documentation
Done

> + *  <li>A fake group implementation when handle type is different from
> + *  HandleTypeContact</li>
> 
> when the handle type *is* HandleTypeContact, surely?
Done

> + * 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)
Done

> + * 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.
Done

> + * As a result, approvers should use the first handler by default.
> 
> ... by default, unless they have a reason to do otherwise.
Done

> 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.
TODO

> + * 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
Done

> + * This corresponds to the _NET_WM_USER_TIME property in EWMH.
> 
> (Unix developers: this corresponds ... in EWMH.)
Done

> 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?
TODO

> + * 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:
Done, did not link the spec, I will do it only in one place so people have a
link to the spec, no need to do it everywhere. Same for link Telepathy wiki
everywhere.

> + * 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".
Done

> + * 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.
Done

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

> + * 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 ..."?
Done

> + * property MAY be zero.
> 
> "may" please, this isn't a specification (unlike telepathy-spec, which uses
> RFC-style MAY/SHOULD/MUST).
> 
Done


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