D10826: Introduce DocumentId class

2018-02-27 Thread Michael Heidelbach
michaelh added inline comments. INLINE COMMENTS > svuorela wrote in documentid.cpp:67 > if you make operator DeviceIdAndInode explicit, you probably end up with a > bit better behavior. > > Else the compiler happily converts a documentid to a deviceidandinode in > order to make various compari

D10826: Introduce DocumentId class

2018-02-26 Thread Sune Vuorela
svuorela added inline comments. INLINE COMMENTS > anthonyfieroni wrote in documentid.h:51-52 > Make a d poniter in exported class. it depends on why it is exported. If it is exported just for unit test (and the header file not installed), then we don't need to have the mental and code wise ove

D10826: Introduce DocumentId class

2018-02-26 Thread Anthony Fieroni
anthonyfieroni added inline comments. INLINE COMMENTS > documentid.h:51-52 > +private: > +DeviceId m_device; > +Inode m_inode; > +}; Make a d poniter in exported class. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10826 To: michaelh, adridg, #baloo, #framewor

D10826: Introduce DocumentId class

2018-02-26 Thread Michael Heidelbach
michaelh added a comment. I have made two errors: 1. Forgot to submit the adapted tests 2. An error in reasoning: I have based this and D10829 on a branch with the adapted tests . As long as the changes are expected to be transparent it is much bet

D10826: Introduce DocumentId class

2018-02-26 Thread Michael Heidelbach
michaelh updated this revision to Diff 28090. michaelh added a comment. - Revert merge with aliases-test REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10826?vs=28005&id=28090 BRANCH flex-class (branched from flexible-docid) REVISION DETAIL https://phab

D10826: Introduce DocumentId class

2018-02-25 Thread Michael Heidelbach
michaelh added a comment. @svuorela: Thank you very much. I will try you suggestion as soon as I'm able to get the tests compiled. INLINE COMMENTS > svuorela wrote in documentid.h:34 > Does it need to be exported? Is things outside of the baloo engine need to > access this? (or is it just a

D10826: Introduce DocumentId class

2018-02-25 Thread Sune Vuorela
svuorela added a comment. A quick review of some quirks and weirdnesses in c++ INLINE COMMENTS > documentid.cpp:67 > +#if(0) > +// Due to operator DeviceIdAndInode(), apparently the following > +// became obsolete. if you make operator DeviceIdAndInode explicit, you probably end up with a bi

D10826: Introduce DocumentId class

2018-02-25 Thread Michael Heidelbach
michaelh added a dependent revision: D10829: Use DocumentId class. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10826 To: michaelh, adridg, #baloo, #frameworks Cc: ashaposhnikov, michaelh, kmorwinski, spoorun, nicolasfella, alexeymin

D10826: Introduce DocumentId class

2018-02-25 Thread Michael Heidelbach
michaelh added a dependency: D10825: Introduce aliases DocId, DeviceId and Inode. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10826 To: michaelh, adridg, #baloo, #frameworks Cc: ashaposhnikov, michaelh, kmorwinski, spoorun, nicolasfella, alexeymin

D10826: Introduce DocumentId class

2018-02-25 Thread Michael Heidelbach
michaelh created this revision. michaelh added reviewers: adridg, Baloo, Frameworks. michaelh added projects: Baloo, Frameworks. michaelh requested review of this revision. REVISION SUMMARY This class shall successively replace the current DocId(quint64) to gain more flexibility. - Account