Review Request: Amazon store WIP patch

Sven Krohlas sven at asbest-online.de
Tue Sep 13 16:36:53 UTC 2011



> On Sept. 13, 2011, 9:03 a.m., Bart Cerneels wrote:
> > src/services/amazon/AmazonParser.h, line 44
> > <http://git.reviewboard.kde.org/r/102596/diff/1/?file=36005#file36005line44>
> >
> >     How big is the responce-document going to be? Should you not rather use QXmlStreamReader rather then keeping the entire DOM in memory and having to wait for end of fetching before parsing.

The response document is quite small, it contains only one page of results. So something like 55 tracks/abums/artists. I think keeping that in memory should be ok. -> Example: http://www.mp3-music-store.de/?apikey=27274503cb405cb1929f353fc507f09c&method=Search&Player=amarok&Location=de&Text=Y3VyZQ==

On my TODO is also a way to fetch the next result page (the API already supports this).


> On Sept. 13, 2011, 9:03 a.m., Bart Cerneels wrote:
> > src/services/amazon/AmazonMeta.cpp, line 87
> > <http://git.reviewboard.kde.org/r/102596/diff/1/?file=36004#file36004line87>
> >
> >     Should not be in the {} block.

Thx, you are right. Fixed. :)


> On Sept. 13, 2011, 9:03 a.m., Bart Cerneels wrote:
> > src/services/magnatune/MagnatuneSettingsModule.cpp, line 37
> > <http://git.reviewboard.kde.org/r/102596/diff/1/?file=36013#file36013line37>
> >
> >     You probably should commit these fixes to master separately so they don't pollute the already huge patch.

Done and already checked in. I'm going to do the same for the changes to the SearchWidget.


- Sven


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


On Sept. 13, 2011, 4:36 p.m., Sven Krohlas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102596/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2011, 4:36 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> Totally WIP! Many, many TODOs and unfinished stuff in the code... pls ignore. ;-)
> 
> I'm currently stuck at the point that my parser created meta items do not show up in the browser... any idea why?
> 
> 
> Diffs
> -----
> 
>   src/browsers/CollectionTreeItemModelBase.cpp 119f90d 
>   src/core-impl/collections/support/MemoryQueryMakerInternal.h d3825ca 
>   src/core/collections/Collection.h 02b0894 
>   src/core/collections/QueryMaker.h dfca79f 
>   src/services/CMakeLists.txt ad14fda 
>   src/services/ServiceCollection.h 7a57aa6 
>   src/services/ServiceMetaBase.cpp 21587d3 
>   src/services/amazon/Amazon.h PRE-CREATION 
>   src/services/amazon/AmazonActions.h PRE-CREATION 
>   src/services/amazon/AmazonActions.cpp PRE-CREATION 
>   src/services/amazon/AmazonCart.h PRE-CREATION 
>   src/services/amazon/AmazonCart.cpp PRE-CREATION 
>   src/services/amazon/AmazonCollection.h PRE-CREATION 
>   src/services/amazon/AmazonCollection.cpp PRE-CREATION 
>   src/services/amazon/AmazonConfig.h PRE-CREATION 
>   src/services/amazon/AmazonConfig.cpp PRE-CREATION 
>   src/services/amazon/AmazonConfigWidget.ui PRE-CREATION 
>   src/services/amazon/AmazonMeta.h PRE-CREATION 
>   src/services/amazon/AmazonMeta.cpp PRE-CREATION 
>   src/services/amazon/AmazonParser.h PRE-CREATION 
>   src/services/amazon/AmazonParser.cpp PRE-CREATION 
>   src/services/amazon/AmazonSettingsModule.h PRE-CREATION 
>   src/services/amazon/AmazonSettingsModule.cpp PRE-CREATION 
>   src/services/amazon/AmazonStore.h PRE-CREATION 
>   src/services/amazon/AmazonStore.cpp PRE-CREATION 
>   src/services/amazon/CMakeLists.txt PRE-CREATION 
>   src/services/amazon/amarok_service_amazonstore_config.desktop PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102596/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Unknown entry/entries after parsing
>   http://git.reviewboard.kde.org/r/102596/s/256/
> 
> 
> Thanks,
> 
> Sven
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20110913/f343bb93/attachment-0001.html>


More information about the Amarok-devel mailing list