----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124794/#review83960 -----------------------------------------------------------
Looks good. I didn't realize making the database global would involve so many changes. autotests/unit/lib/filefetchjobtest.cpp (line 91) <https://git.reviewboard.kde.org/r/124794/#comment58157> What is this used for? src/engine/database.h (line 37) <https://git.reviewboard.kde.org/r/124794/#comment58151> The Database class isn't a QObject. This is not required. src/engine/database.h (line 47) <https://git.reviewboard.kde.org/r/124794/#comment58152> Could you please just check if m_env is not 0. The extra variable isn't required. src/engine/database.cpp (line 60) <https://git.reviewboard.kde.org/r/124794/#comment58158> Considering that you're always calling `isOpen` before open, maybe we can simplify the code by checking if the db is already open and then returning? That way you don't need to put an 'isOpen' everywhere. src/engine/global.h (line 32) <https://git.reviewboard.kde.org/r/124794/#comment58160> Do you think we could rename this function to something a bit more descriptive? Perhaps `globalDatabaseInstance`. We should also put comments on why one needs this global instance . src/engine/global.cpp (line 3) <https://git.reviewboard.kde.org/r/124794/#comment58153> I think you mispelled your name :) src/file/extractor/main.cpp (line 49) <https://git.reviewboard.kde.org/r/124794/#comment58154> The code in the extractor is getting quite complex. Perhaps we can just move the "BALOO_DB_PATH" environment variable in the globals file. That way we do not need to to remember to open/close another database. src/lib/query.cpp (line 209) <https://git.reviewboard.kde.org/r/124794/#comment58156> This means the prefix hash and other possible variables will be generated each time. I'm not sure if that is good / bad. I guess it is fine for now. - Vishesh Handa On Aug. 17, 2015, 6:32 p.m., Ashish Bansal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124794/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2015, 6:32 p.m.) > > > Review request for Baloo, Bhushan Shah and Vishesh Handa. > > > Bugs: 350247 > http://bugs.kde.org/show_bug.cgi?id=350247 > > > Repository: baloo > > > Description > ------- > > Earlier two Database instances were being created and once one of them was > destroyed, it leads to invalidation of handles of other instance too. Now a > global single instance is being created and is destroyed when process ends. > > > Diffs > ----- > > autotests/unit/lib/db.cpp af34025 > autotests/unit/lib/filefetchjobtest.cpp 2d53c8c > src/engine/CMakeLists.txt 109c19e > src/engine/database.h 1556f0a > src/engine/database.cpp 9dd7327 > src/engine/global.h PRE-CREATION > src/engine/global.cpp PRE-CREATION > src/file/extractor/app.cpp 1575f05 > src/file/extractor/main.cpp 3dfb28d > src/file/main.cpp bd162a7 > src/lib/CMakeLists.txt 7bf52e0 > src/lib/db.h 4b87345 > src/lib/db.cpp c9a11f2 > src/lib/file.cpp 13dff88 > src/lib/query.cpp 99fb91a > src/lib/searchstore.cpp 0df20b9 > src/lib/taglistjob.cpp 1abe071 > src/tools/baloo-monitor/monitor.cpp e78e287 > src/tools/balooctl/main.cpp 0736b0c > src/tools/balooshow/main.cpp 02af838 > autotests/unit/lib/CMakeLists.txt 871bf8d > > Diff: https://git.reviewboard.kde.org/r/124794/diff/ > > > Testing > ------- > > Plasma Mediacenter works fine. > > > Thanks, > > Ashish Bansal > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
