Review Request 125537: Use largest timestamp in subdirectory as resource directory timestamp.

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125537/#review86416
-----------------------------------------------------------


Nice and original solution, I like it.

Just some last fixes I would recommend, mostly to comments.


src/sycoca/kbuildsycoca.cpp (line 174)
<https://git.reviewboard.kde.org/r/125537/#comment59542>

    0 would have worked, it means 1970 :-)
    
    I would be less worried about portability if you used 0 here, since that's really good enough, 19 70 is long gone.
    
    (then you don't need <stdint.h>)



src/sycoca/kbuildsycoca.cpp (line 303)
<https://git.reviewboard.kde.org/r/125537/#comment59547>

    0



src/sycoca/ksycocautils_p.h (line 28)
<https://git.reviewboard.kde.org/r/125537/#comment59546>

    remove



src/sycoca/ksycocautils_p.h (line 40)
<https://git.reviewboard.kde.org/r/125537/#comment59545>

    change comment to "helper function for visitResourceDirectory" ? it's not part of the "internal API", it's just part of the implementation of visitResourceDirectory.



src/sycoca/ksycocautils_p.h (line 60)
<https://git.reviewboard.kde.org/r/125537/#comment59544>

    well, technically these templates can do anything else; the comment should be more generic, like "apply the visitor in the given dir, and its subdirs if it's expected to have subdirs".
    
    Also, the return value should be documented.
    AFAICS it's "return true if the visitor returned true for every visited directory, otherwise return false", right?



src/sycoca/ksycocautils_p.h (line 69)
<https://git.reviewboard.kde.org/r/125537/#comment59543>

    this comment doesn't make sense anymore ;)


- David Faure


On Oct. 6, 2015, 9:53 a.m., Xuetian Weng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125537/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 9:53 a.m.)
> 
> 
> Review request for KDE Frameworks, Albert Astals Cid and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> -------
> 
> In 0e4d7247e27f82a9b79b19dbceb35b603baceb76, a new feature saving directory mtime in ksycoca db is introduced.
> 
> In the related code, TimestampChecker() will check the mtime recursively, but the building process will only read the top level directories' mtime, which causes ksycoca db being recreated again and again.
> 
> I hit this because my ~/.config/menus/applications-merged mtime newer than ~/.config/menus. And plasmashell is running with 100% since it's creating ksycoca db repeatedly.
> 
> This patch change the building db procedure to use the same logic as in TimestampChecker(), so building and checking are consistent now.
> 
> 
> Diffs
> -----
> 
>   autotests/ksycocatest.cpp 67f0700 
>   src/sycoca/ksycocautils_p.h c14057b 
>   src/sycoca/ksycoca.cpp 5209389 
>   src/sycoca/kbuildsycoca.cpp 00aaac3 
> 
> Diff: https://git.reviewboard.kde.org/r/125537/diff/
> 
> 
> Testing
> -------
> 
> Now a sub directory with larger timestamp than parent will not cause ksycoca data being recreated again and again.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20151006/f33c032b/attachment-0001.html>