[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Moving Baloo and Baloo-widgets into KDE SC
From: Vishesh Handa <me () vhanda ! in>
Date: 2014-01-20 17:43:13
Message-ID: 4101628.tCVWWq3Ocv () vlap
[Download RAW message or body]
On Sunday 19 January 2014 15:56:17 David Edmundson wrote:
> Code Review of baloo/file/lib
>
> ==file.cpp
>
> Should it override type() from Item and set it to "File" ?
>
I'll probably removing this whole type thing. I'm not sure.
>
> ==filemodifyjob.cpp.
>
> The code won't unset a rating, comments or tags on multiple files.
> You update Xapian ok, but you're not calling fsetxattr().
>
Right. I think I'll need to re-architecture the internals. There needs to be a
way to differentiate between removing the rating and the user not setting
them.
Eg - File f(someUrl); f.setRating(5); save f;
In this case the comment and tags would be removed even though they should
not.
> The d pointer leaks?
>
Fixed d-pointer.
> ==DB.cpp
> SQLITE3-> SQLITE
>
Fixed.
> ==general
>
> Are you planning on putting all the Xapian queries in a new thread in
> the future? If not having a KJob API doesn't have any benefit does it?
>
Not right now. Xapian blocks for a very very small amount. In the future if
required it can be moved to another thread.
> I would suggest you namespace your header guards
> i.e
> #ifndef FILE_H -> #ifndef BALOO_FILE_H
> The current ones are very generic, it will be very easy to accidentally
> clash.
>
Thanks. Fixed.
> David
>
>
> David
--
Vishesh Handa
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic