poboiko added inline comments.

INLINE COMMENTS

> bruns wrote in unindexedfileiteratortest.cpp:100
> Make this plain members, not pointers.
> Also, one temporary dir is enough, you can put the db and the test tree 
> side-by-side.

I wanted both DB and directory tree to be recreated from scratch for each test, 
since tests modify DB or rename folders, and thus can affect each other. That's 
why I made them to be pointers.

> unindexedfileiterator.cpp:118
>      auto fileMTime = fileInfo.metadataChangeTime().toTime_t();
>      if (timeInfo.cTime != fileMTime) {
>          m_cTimeChanged = true;

I wanted to note that this line is messed up a bit: either `fileMTime` should 
be renamed to `fileCTime`, or even removed, just like three lines above. 
(it looks like the variable was introduced some time ago inside `#ifdef QT < 
5.10.0`, which was removed recently with Qt dependency bump).

Since that's just some cosmetic change, I can change it in this patch as well. 
Or should I make a new one?

> bruns wrote in unindexedfileiterator.cpp:122
> thats wrong:
> 
>   $:/tmp> touch dir
>   $:/tmp> stat dir
>     File: dir
>     Size: 0               Blocks: 0          IO Block: 4096   regular empty 
> file
>   Device: 3fh/63d Inode: 1892961     Links: 1
>   Access: (0644/-rw-r--r--)  Uid: ( 1000/ sbruens)   Gid: (  100/   users)
>   Access: 2019-06-03 19:21:57.764626372 +0200
>   Modify: 2019-06-03 19:21:57.764626372 +0200
>   Change: 2019-06-03 19:21:57.764626372 +0200
>    Birth: 2019-06-03 19:21:57.764626372 +0200
>   $:/tmp> mv dir  dir_renamed
>   $:/tmp> stat dir_renamed 
>     File: dir_renamed
>     Size: 0               Blocks: 0          IO Block: 4096   regular empty 
> file
>   Device: 3fh/63d Inode: 1892961     Links: 1
>   Access: (0644/-rw-r--r--)  Uid: ( 1000/ sbruens)   Gid: (  100/   users)
>   Access: 2019-06-03 19:21:57.764626372 +0200
>   Modify: 2019-06-03 19:21:57.764626372 +0200
>   Change: 2019-06-03 19:22:07.096601727 +0200
>    Birth: 2019-06-03 19:21:57.764626372 +0200

Right, sorry, I've mixed them up. Damn russian locale.

> bruns wrote in unindexedfileiterator.cpp:130
> The comment does not match - the mtime changes when something **in** the 
> directory is modified, when the directory is renamed, the ctime changes.

The comment is not mine - but that's precisely what it says. We don't want to 
reindex a directory every time something inside it modifies.

REPOSITORY
  R293 Baloo

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

To: poboiko, #frameworks, #baloo, bruns
Cc: kde-frameworks-devel, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

Reply via email to