[Bug 31195] Implement models

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Jan 3 17:26:20 CET 2011


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

--- Comment #8 from Mateu Batle <mateu.batle at collabora.co.uk> 2011-01-03 08:26:20 PST ---
OK, most of the changes have been done, check
http://git.collabora.co.uk/?p=user/mbatle/telepathy-qt4.git;a=shortlog;h=refs/heads/review1
Start review from commit b292753a30759c3656a693a1ce41bf5f0ecb2090

Things missing:

- Add tests for all new classes, the test coverage (make lcov-check) should be
at least yellow.

- Make sure the examples won't break built if QtDeclarative is not installed.

Some answers to your questions:

> +bool AccountsModelItem::setData(int role, const QVariant &value)
> +{
> +    switch (role) {
> +    case AccountsModel::EnabledRole:
> +        setEnabled(value.toBool());
> +        return true;
> +    case AccountsModel::RequestedPresenceRole:
> +        setStatus(value.toString());
> +        return true;
> +    case AccountsModel::RequestedPresenceStatusMessageRole:
> +        setStatusMessage(value.toString());
> +        return true;
> It would be good to be able to set the presence status and status message at
> the same time, 
> avoiding 2 dbus calls when someone wants to set both at the same time.

setData is a standard function of models, so if we want to represent the type,
status and status message as roles, then we should allow setting them
individually in despite of the worse performance.

> +    Q_INVOKABLE void setPresence(int type, const QString &status, const
> QString &statusMessage);
> I would avoid exposing setStatus and setStatusMessage in favor of only exposing
> setPresence or better phrased setCurrentPresence.

This can be done, although in most cases it would not provide any improvement
IMHO.

> +Q_SIGNALS:
> +    void connectionStatusChanged(const QString &accountId, int status);
> What is the point in having this signal? Shouldn't it be emitted as changed()
> as the other properties do? Any special case here?

Yes, that is used in several places in our client, in case of connection status
changed is very different than from a change in some properties. It is used for
example to become ready the connection, upgrade the contacts, etc.

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