----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109965/#review30976 -----------------------------------------------------------
looking very good. a few comments below, but this should be ready soon, i imagine. assetimporters/database-common/database.cpp <http://git.reviewboard.kde.org/r/109965/#comment22993> would be nicer imho to cache this value after doing a look up once in the database. so if this the "KDE" partner, do a lookup for the KDE partner and then cache the result for later re-use (we can assume it won't change in the database :) assetimporters/database-common/database.cpp <http://git.reviewboard.kde.org/r/109965/#comment22994> awesome, sizes! very stoked to see that. assetimporters/database-common/database.cpp <http://git.reviewboard.kde.org/r/109965/#comment22995> may as well move this after the check to line 481 assetimporters/kdeartwork-wallpapers/kdewallpapersdatabase.cpp <http://git.reviewboard.kde.org/r/109965/#comment22996> same as with books: query the db for the value and then assign it to the members (m_licenseId, m_partnerId) would be better assetimporters/obs/packagedatabase.cpp <http://git.reviewboard.kde.org/r/109965/#comment22997> spacing: foreach ( assetimporters/projectgutenberg/src/gutenbergdatabase.cpp <http://git.reviewboard.kde.org/r/109965/#comment22999> this is more partnerId than partnerQuery? assetimporters/projectgutenberg/src/gutenbergdatabase.cpp <http://git.reviewboard.kde.org/r/109965/#comment22998> this can be done once and then the value cached assetimporters/projectgutenberg/src/gutenbergdatabase.cpp <http://git.reviewboard.kde.org/r/109965/#comment23000> as with partnerQuery above, this is probably more accurately named languageId assetimporters/projectgutenberg/src/gutenbergdatabase.cpp <http://git.reviewboard.kde.org/r/109965/#comment23001> this could also possibly be cached, e.g.: int id = m_languageIds.value(lang); if (id < 1) { QSqlQuery query; .. do the query .. id = res.toInt(); m_languageIds.insert(lang, id); } return id; assetimporters/projectgutenberg/src/gutenbergdatabase.cpp <http://git.reviewboard.kde.org/r/109965/#comment23002> same as methods above - Aaron J. Seigo On April 12, 2013, 6:45 p.m., Giorgos Tsiapaliokas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109965/ > ----------------------------------------------------------- > > (Updated April 12, 2013, 6:45 p.m.) > > > Review request for Plasma. > > > Description > ------- > > This patch > > * removes the duplicated code in assetimporters > * adds asset's size into the db > * and fixes a few small issues > > > Diffs > ----- > > assetimporters/CMakeLists.txt 24e76a0 > assetimporters/database-common/channelscatalog.h 5d39c02 > assetimporters/database-common/channelscatalog.cpp 6ca0aef > assetimporters/database-common/database.h 9883216 > assetimporters/database-common/database.cpp e860bdd > assetimporters/kdeartwork-wallpapers/CMakeLists.txt 56d19b9 > assetimporters/kdeartwork-wallpapers/database.h 6991758 > assetimporters/kdeartwork-wallpapers/database.cpp d75cdda > assetimporters/kdeartwork-wallpapers/kdewallpapers-channels.ini b22f395 > assetimporters/kdeartwork-wallpapers/kdewallpapersdatabase.h PRE-CREATION > assetimporters/kdeartwork-wallpapers/kdewallpapersdatabase.cpp PRE-CREATION > assetimporters/kdeartwork-wallpapers/main.cpp 708a949 > assetimporters/obs/CMakeLists.txt 2dbcd42 > assetimporters/obs/channelscatalog.h PRE-CREATION > assetimporters/obs/channelscatalog.cpp PRE-CREATION > assetimporters/obs/packagedatabase.h 99f4e17 > assetimporters/obs/packagedatabase.cpp ae43b8e > assetimporters/projectgutenberg/CMakeLists.txt b86cc49 > assetimporters/projectgutenberg/src/CMakeLists.txt 2d48e9c > assetimporters/projectgutenberg/src/database.h 8dba0ba > assetimporters/projectgutenberg/src/database.cpp 75cba69 > assetimporters/projectgutenberg/src/gutenbergdatabase.h PRE-CREATION > assetimporters/projectgutenberg/src/gutenbergdatabase.cpp PRE-CREATION > assetimporters/projectgutenberg/src/main.cpp 46f2340 > sql/bodega.sql 44f8641 > > Diff: http://git.reviewboard.kde.org/r/109965/diff/ > > > Testing > ------- > > I haven't noticed regression. > > > Thanks, > > Giorgos Tsiapaliokas > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
