https://bugs.gpodder.org/show_bug.cgi?id=1579

--- Comment #3 from Thomas Perl <[email protected]> 2012-05-28 15:15:14 BST ---
(In reply to comment #2)
> Created attachment 710 [details]
> 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.
> 
> 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.
> 
> 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.

Thanks for the updated patch, some feedback:

 - The configuration section should be named "device_sync", not "sync"
   fssync_channel_subfolder should be named "one_folder_per_podcast"
   only_sync_not_played should be named "skip_played_episodes"
   mp3_player_delete_played should be named "delete_played_episodes"
   mp3_player_max_filename_length should be named "max_filename_length"
   the on_sync_* options should be grouped into "after_sync", e.g.:

 'device_sync': {
     # ...
     'after_sync': {
         'mark_episodes_played': True,
         'delete_episodes': True,
         'sync_disks': False,
     },
  },

 (the old configuration names were like that for historic reasons -
  this new rewrite is a good opportunity to get rid of my bad
  historic decisions ;)

 - ACTION_NONE, ACTION_ASK, ACTION_MINIMIZED, ACTION_ALWAYS should
   probably be removed from DeviceTypeActionList

 - Be aware of trailing whitespace (remove it if possible, otherwise
   I will do it when I merge the patch, so not really a problem)

 - In "on_sync_to_ipod_activate", you have backslashes (\) for line
   continuation. This is not required, and PEP-8 suggests against it,
   and while other code in gPodder has this style, I want to get rid
   of it (again, one of my bad historic style decisions ;). Also, in
   some cases, the lines are longer than 80 chars which (while not a
   hard rule) should be avoided where possible). Again, if you do not
   want to deal with that, I can deal with that reformatting when
   merging the patch.

 - Not "directory_is_writable( path):" but "directory_is_writable(path):"
   (yes, this is again something that we shouldn't have done in old code ;)

 - disable_pre_sync_conversion in sync.py is not available in the config
   (IIRC we said that for the first version, we don't want to have file
   conversion - is that correct? if so, feel free to remove the code in
   sync.py that relates to conversion [libconverter et al] and also remove
   libconverter.py itself for now)

 - When I click on Extras -> Sync to device (with no device configured),
   I should either get an error message or the menu should not be shown
   at all.

 - I still get an error when syncing (remove sender=):

Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 551, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 504, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/home/thp/src/gpodder/src/gpodder/gtkui/desktop/sync.py", line 161, in
sync_thread_func
    device.add_sync_tasks(episodes, force_played=force_played)
  File "/home/thp/src/gpodder/src/gpodder/sync.py", line 208, in add_sync_tasks
    logger.info('Excluding %s from sync', track.title, sender=self)
  File "/usr/lib/python2.7/logging/__init__.py", line 1140, in info
    self._log(INFO, msg, args, **kwargs)
TypeError: _log() got an unexpected keyword argument 'sender'


Do you want me to clean up the patch a bit and send it back to you so you can
continue working on it, or do you want to clean up the patch and re-submit it?
Thanks for the work so far, you're doing a great job, and I hope to be able to
merge the patch soon! :)

-- 
Configure bugmail: https://bugs.gpodder.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
_______________________________________________
gPodder-Bugs mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/gpodder-bugs

Reply via email to