[Merge] lp:~boiko/history-service/cached_contact_matching into lp:history-service

Tiago Salem Herrmann tiago.herrmann at canonical.com
Thu Oct 8 21:43:53 UTC 2015


Review: Needs Fixing

Looks good. 
Just a few remarks.

Diff comments:

> 
> === added file 'daemon/history-daemon.conf'
> --- daemon/history-daemon.conf	1970-01-01 00:00:00 +0000
> +++ daemon/history-daemon.conf	2015-10-08 19:36:10 +0000
> @@ -0,0 +1,7 @@
> +description "history-daemon"
> +author "Gustavo Pichorim Boiko <gustavo.boiko at canonical.com>"
> +
> +start on started address-book-service
> +stop on runlevel [06]
> +
> +exec dbus-send --session --dest=com.canonical.HistoryService /com/canonical/HistoryService org.freedesktop.DBus.Peer.GetMachineId

can you add a comment explaining why history-daemon isn't called directly?

> 
> === modified file 'plugins/sqlite/sqlitehistoryplugin.cpp'
> --- plugins/sqlite/sqlitehistoryplugin.cpp	2015-10-08 19:36:10 +0000
> +++ plugins/sqlite/sqlitehistoryplugin.cpp	2015-10-08 19:36:10 +0000
> @@ -841,6 +876,12 @@
>          QString accountId = query.value(0).toString();
>          QString threadId = query.value(1).toString();
>          QString eventId = query.value(2).toString();
> +
> +        // ignore events that don't have a threadId or an eventId
> +        if (threadId.trimmed().isEmpty() || eventId.trimmed().isEmpty()) {

Just wondering if we should really exclude threads with no events. They might be needed for the drafts support.

> +            continue;
> +        }
> +
>          event[History::FieldType] = (int) type;
>          event[History::FieldAccountId] = accountId;
>          event[History::FieldThreadId] = threadId;


-- 
https://code.launchpad.net/~boiko/history-service/cached_contact_matching/+merge/272188
Your team Ubuntu Phablet Team is subscribed to branch lp:history-service.



More information about the Ubuntu-reviews mailing list