[gpodder] Re: gPodder 3.1.0 "The Discipline of D.E." released

Hi Joseph,

(sorry for taking again very long to respond, but I wanted to allocate
enough time again to make sure I can review the patch and give some
feedback)

2012/5/20 Joseph Wickremasinghe <jnwickremasinghe@xxxxxxxxx>:
> Thanks for your comments. After my last email, I switched to a 'one task per 
> episode' approach and made some further progress. I'm including a patch to 
> add device sync to gpodder which uses the 'Downloads' tab to show progress.

Thanks - this looks good. I've left some feedback on the bug report
where you submitted the patch.

> The synchronization process opens the device and creates a 'SyncTask' object 
> for each episode synchronized. The SyncTask object is derived from 
> DownloadTask, overriding the run() method. The task is then registered with 
> the download status model and added to the download queue, which then 
> executes it and updates the GUI in the same way as for downloads.

Yes, that sounds good and should work. For later (after merging or
just before), we should probably rename the classes and the UI parts
so that is clear to the user that the downloads tab is also used for
other progress information (and also in the code so that developers
reading the code can quickly find their way through the codebase).

> Please let me know your comments. :) There is probably some code refactoring 
> that could be done, perhaps a reworking of the Device & SyncTask objects, 
> too. Presently the Device adds the Task objects to the queue, but when the 
> tasks are executed they call the add_track method on the device to actually 
> copy the files over. Probably not the cleanest way to do it, but this is 
> still a work-in-progress :) This is currently only implemented for a 
> filesystem-based mp3 player.

See the comments in the bug report. Please also tell me if you'd
rather like me to have a go through the patch and do some clean-ups
that I think are necessary (mostly just code style, etc..) or if you
would like to do this yourself (I'm happy with both approaches, and
don't want to annoy you with requests for reformatting the code ;).

> I can certainly work on making this an extension as well, but it was easier 
> for me to finalize the interface and then figure out how to make it an 
> extension.

Yep, I guess the first step could be to integrate this, and then
re-work this as an extension. At this point, I'm quite happy if we
could get it in as such (for the Gtk UI only) and then later work on
refactoring it up to a point where it can be used by the CLI UI as
well (maybe even Web UI).

Looking forward to your comments and the next patch! Thanks for
working on this, it's really appreciated :)

Thanks,
Thomas

Other related posts: