Review Request 109965: Refactor assetimporters

[Giorgos Tsiapaliokas]

> On April 13, 2013, 2:12 a.m., Aaron J. Seigo wrote:
> > looking very good. a few comments below, but this should be ready soon, i imagine.

Regarding the cache of the partnerId and licenseId, I cache those values in the *init method.
I don't follow you in this one. What is the difference between caching the value inside the
method and caching the value in the *init method?


> On April 13, 2013, 2:12 a.m., Aaron J. Seigo wrote:
> > assetimporters/projectgutenberg/src/gutenbergdatabase.cpp, line 326
> > <http://git.reviewboard.kde.org/r/109965/diff/1/?file=138225#file138225line326>
> >
> >     this is more partnerId than partnerQuery?

yes you can call it a partnerId, but I didn't. Because partnerQuery is a virtual method of databasecommon
and I wanted to emphasize the fact that *this* partner has changed.

So I believe we should leave the method as partnerQuery or to rename all of them into partnerId.
We have to emphasize the change in the value(if it changes at all), no?


> On April 13, 2013, 2:12 a.m., Aaron J. Seigo wrote:
> > assetimporters/projectgutenberg/src/gutenbergdatabase.cpp, line 341
> > <http://git.reviewboard.kde.org/r/109965/diff/1/?file=138225#file138225line341>
> >
> >     as with partnerQuery above, this is probably more accurately named languageId

same answer as the partnerQuery. :)


> On April 13, 2013, 2:12 a.m., Aaron J. Seigo wrote:
> > assetimporters/projectgutenberg/src/gutenbergdatabase.cpp, line 357
> > <http://git.reviewboard.kde.org/r/109965/diff/1/?file=138225#file138225line357>
> >
> >     same as methods above

same answer as above :)


- Giorgos


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109965/#review30976
-----------------------------------------------------------


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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130420/92113155/attachment.html>