D10825: Introduce aliases DocId, DeviceId and Inode

Nathaniel Graham noreply at phabricator.kde.org
Mon Feb 26 00:11:31 UTC 2018


ngraham added a comment.


  Nice job, looks good. No compilation warnings for me, no test failures, and all functionality that I tested still works. I found a few minor formatting issues; see the below comments.

INLINE COMMENTS

> idtreedb.cpp:76
>  
> -QVector<quint64> IdTreeDB::get(quint64 docId)
> -{
> -    MDB_val key;
> -    key.mv_size = sizeof(quint64);
> -    key.mv_data = static_cast<void*>(&docId);
> +            QVector<DocId> IdTreeDB::get(DocId docId)
> +            {

Unintentional indentation?

> result.h:44
>  
> -    quint64 id() const;
> +    Baloo::DocId id() const;
>      QVariantMap map() const;

Can we `#include "idutils.h"` on this file too, so we don't have to use the `Baloo::` namespace prefix here and in the implementation file?

> main.cpp:114
>      for (QString url : urls) {
> -        quint64 fid = 0;
> +        Baloo::DocId fid = 0;
>          if (url.startsWith(QLatin1String("file:"))) {

Ditto.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D10825

To: michaelh, adridg, #baloo, #frameworks
Cc: ngraham, alexeymin, ashaposhnikov, michaelh, spoorun, nicolasfella
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180226/329f978a/attachment.html>


More information about the Kde-frameworks-devel mailing list