Re: Review Request: Statistics Synchronization: final review request

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107348/#review22353 --- This review has been submitted with commit 562ce2e782e5426f9ab

Re: Review Request: Statistics Synchronization: final review request

On Mon, Nov 19, 2012 at 09:11:18PM +0100, Matěj Laitl wrote: > > > > You have this restriction in StatSync::Provider, and according to > > yourself. I seem to have swallowed the end of the sentence. It should have gone like this: ...according to yourself, existing QueryMakers won't work unless c

Re: Review Request: Statistics Synchronization: final review request

On 19. 11. 2012 Edward Hades Toroshchin wrote: > On Sun, Nov 18, 2012 at 10:43:07PM -, Matěj Laitl wrote: > > [wrt BlockingQueryMaker created from code in > > StatSyncing::CollectionProvider] > > > > Cannot be done. There couldn't be any "BlockingQueryMaker only created > > in the main thread"

Re: Review Request: Statistics Synchronization: final review request

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107348/ --- (Updated Nov. 19, 2012, 3:18 p.m.) Review request for Amarok and Myriam Sc

Re: Review Request: Statistics Synchronization: final review request

On 19. 11. 2012 Edward Hades Toroshchin wrote: > Matěj Laitl wrote: > > So, what's your "ready to be merged" opinion on this, Edward? > > I don't like it, but just go ahead, there doesn't seem to be anything that > could be improved without improving all the rest of Amarok first. Okay. I will for

Re: Review Request: Statistics Synchronization: final review request

On Sun, Nov 18, 2012 at 10:43:07PM -, Matěj Laitl wrote: > Cannot be done. There couldn't be any "BlockingQueryMaker only created > in the main thread" restriction (which would make it useless) and > without it, the code wouldn't work. You have this restriction in StatSync::Provider, and accor

Re: Review Request: Statistics Synchronization: final review request

On Sun, Nov 18, 2012 at 10:30:11PM -, Matěj Laitl wrote: > Well, the method is (and should only be) called by existing > StatSyncing code Then the existing code is the user of the API. The code exists, doesn't mean it will stay unmodified for the rest of times. > the "users" of the API would

Re: Review Request: Statistics Synchronization: final review request

> On Nov. 17, 2012, 7:59 p.m., Edward Hades Toroshchin wrote: > > Haven't run it yet, but at least it compiles okay :) > > Matěj Laitl wrote: > Thanks for the attentive review, I didn't expect somebody to dive that > deep and I'm grateful. > > Matěj Laitl wrote: > So, what's your "read

Re: Review Request: Statistics Synchronization: final review request

> On Nov. 17, 2012, 7:59 p.m., Edward Hades Toroshchin wrote: > > src/statsyncing/collection/CollectionProvider.cpp, lines 126-132 > > > > > > I think this should be implemented in QueryMaker as a blocking API > >

Re: Review Request: Statistics Synchronization: final review request

> On Nov. 17, 2012, 7:59 p.m., Edward Hades Toroshchin wrote: > > src/services/lastfm/SynchronizationAdapter.cpp, lines 38-43 > > > > > > This should be Qt::AutoConnection. Otherwise it will deadlock if > > artists(

Re: Review Request: Statistics Synchronization: final review request

> On Nov. 17, 2012, 7:59 p.m., Edward Hades Toroshchin wrote: > > Haven't run it yet, but at least it compiles okay :) > > Matěj Laitl wrote: > Thanks for the attentive review, I didn't expect somebody to dive that > deep and I'm grateful. So, what's your "ready to be merged" opinion on th

Re: Review Request: Statistics Synchronization: final review request

> On Nov. 17, 2012, 7:59 p.m., Edward Hades Toroshchin wrote: > > src/services/lastfm/SynchronizationAdapter.cpp, lines 38-43 > > > > > > This should be Qt::AutoConnection. Otherwise it will deadlock if > > artists(

Re: Review Request: Statistics Synchronization: final review request

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107348/ --- (Updated Nov. 18, 2012, 5:12 p.m.) Review request for Amarok and Myriam Sc

Review Request: Statistics Synchronization: final review request

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107348/ --- Review request for Amarok and Myriam Schweingruber. Description --- T