-
-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(video): fetch youtube thumbnail for media_type=video #80
base: gnome-40-42
Are you sure you want to change the base?
feat(video): fetch youtube thumbnail for media_type=video #80
Conversation
Thanks, this looks good! Often images from APOD are not high resolution either, so a 720p image should be fine too. It's definitely better than nothing... |
So what should I do next? Should I add a toggle in the settings for this functionality? I am sitting at gnome 42, so I was planning to rebase onto master and submit another PR once the job is done |
I suppose I should also consider the quality setting when replacing the URL |
Well, I don't think the toggle in the settings is necessary, there are already quite a few switches. However there may be some privacy implications if we download from YouTube, so maybe it is necessary after all? Yes, you can rebase for master (actually it's main) in a second PR. Yes, the quality setting should be considered, but if the best quality is 720p, I'd say don't bother (720p is not exactly hi-res, is it?). |
3a51fdb
to
7a2d1d3
Compare
Also, there would be great to have some functional testing. Now I can't really test this PR. I've tried to set local time or pin the wallpaper. Seems like it would be fairly easy to mock apod api, gsettings and local time |
POC #79