openzim / youtube

Create a ZIM file from a Youtube channel/username/playlist
GNU General Public License v3.0
46 stars 26 forks source link

Add feature to filter videos by date #68

Closed deborahlow97 closed 4 years ago

deborahlow97 commented 4 years ago

Fixes #31

Context

Approach

rgaudin commented 4 years ago

Great! It's working fine. Couple things I'd love though:

deborahlow97 commented 4 years ago

Great! It's working fine. Couple things I'd love though:

  • DateRange is very permissive but can still fail with garbage value. Could you protect that and make sure user gets an informative message in case of incorrect value? I'd prefer that to happen in run() so you could just store it in __init__ and convert/fail to DateRange in run
  • Logs should display the daterange as well ; in the same way we display the nb of playlists/videos.
  • in filter_videos_not_within_date_range, I see no need for your date_range variable. Can we get rid of it? Actually, I think you used it for sementic purposes and that makes me think we should maybe store the date range in a self.date_range instead. The parameter can stay as it is, as it implies youtube-dl compatibility.

👍 Alright, will do! Thanks for the informative feedback 😄

deborahlow97 commented 4 years ago

Currently, if no --dateafter is provided, INFO:videos in date range: 0001-01-01 - CURRENT_DATE_IN_YYYY-MM-DD would be logged.

rgaudin commented 4 years ago

Thank you @deborahlow97 !