search

skeeto / youtube-dl-emacs

Emacs youtube-dl download manager
The Unlicense
104 stars 11 forks source link

Add to ELPA/MELPA? #3

Open Ambrevar opened 6 years ago

Ambrevar commented 6 years ago

I was just wondering if you'd consider adding this package to ELPA/MELPA :)

skeeto commented 6 years ago

I don't really have much reason to object to it. It's just not something I think about since I no longer use ELPA or MELPA myself.

My only reason against is that I don't really want to provide support for this package. It exists only to scratch a personal itch and vice (consuming too much YouTube). Bugs don't matter to me as long as they don't get in my way, and I don't want to extend the scope (e.g. to support sites other than YouTube, or become a generic download manager).

Ambrevar commented 6 years ago

I see, no worries.

I don't use ELPA / MELPA either, that said I think it's a practical way to publicize packages. It reduces the risk of someone not finding your package and re-implementing everything from scratch.

stardiviner commented 3 years ago

@skeeto Hi, I would like to maintain this package. And I added some code to this package. I want to get permission from you to public this package (my fork) to MELPA. What do you think?

https://github.com/stardiviner/youtube-dl.el

skeeto commented 3 years ago

Thanks for offering to take over maintenance, @stardiviner. Since you're willing to do the work, I'm interested in handing it off. However, I have some concerns about the current state of your master branch:

Even though I haven't made any commits to this package for nearly 3 years, it's not abandoned and I still use it literally every day as a complement to Elfeed. It hasn't needed attention since it's worked perfectly for me all these years — a well-oiled, well-tuned machine. I'd like for that to continue even if new features are introduced. Addressing these items would demonstrate that you're ready for the job.

stardiviner commented 3 years ago

Thanks for your reviewing of my code, your questions and suggestions. I'm glad.

  • [X] It doesn't byte-compile cleanly (try running "=make="). There are currently two warnings: a missing require and an unused argument.

I fixed this two byte-compile warnings

  • [X] When I start up Emacs and "M-x youtube-dl-list" nothing happens, which is confusing. I'm probably the last person who should be confused by any behavior of this package. I think it's the new autoclose thing?

Yes, youtube-dl-list buffer will be auto closed when there is no items downloading. I added an local timer to do this.

  • [ ] When I add a video to the list there's a long freeze, which was pretty irritating. I suspect it's the call-process calls you've introduced to grab metadata. Since they're so slow, these calls should at least be asynchronous so that it doesn't lock up Emacs. (Shame on me for using call-process in the first place to fetch playlist information, though I do hardly use this feature.) Alternatively: Consider if it's really necessary to do it at all given the cost. It takes time and causes extra HTTP traffic for YouTube.

Indeed I will consider to use better way to retrieve video metadata. Might consider improve code in future.

  • [ ] It would be nice if "make simulate" still worked. I've used it to test UI changes without actually downloading any videos. I tried to use it to test drive your changes but found that it didn't work anymore. (Feel free to make it less janky, too, if that's the reason why you're not using it!)

I already forget which function is to "make simulate" now.... Sorry. Can you tell me which function it is?

  • [ ] My version accepts bare YouTube video IDs at the "URL" prompt since only YouTube is supported. Your version has broader support, so it makes sense that this might no longer be supported. However, I tried anyway and got an Emacs Lisp type error rather than a more useful message.

Indeed, I try to make it support youtube-dl directly for all supported websites. It will be more useful. I forget the detailes about changes. But I guess some part of changes are because of this reason. I have not tested all websites. Just myself used few videos websites are fine.

Can you report the error backtrace? Curious which part of code is wrong. I will fix it.

  • [ ] Your version requires at least Emacs 27.1, a release that is less than a year old. My version requires Emacs 24.3 (released March 2013) — so old that it's hardly worth mentioning. I generally try to avoid depending on such new versions of Emacs unless there's a good reason to do so (critical new feature or fix) since not everyone follows the bleeding edge. For instance, Debian stable is still on 26.1 and so cannot run your version of youtube-dl.el. If you believe it's really necessary to use Emacs 27, at least note it in the README and update the Package-Requires. (Example: url-domain might be worth such a requirement, but string-empty-p and when-let certainly are not.)

Emmm, this is indeed a reason. I'm not very familar with API compatibility.

About the Emacs version compatibility issue, I have an idea, separate it to stable and unstable version start from your last commit. What do you think?

Even though I haven't made any commits to this package for nearly 3 years, it's not abandoned and I still use it literally every day as a complement to Elfeed. It hasn't needed attention since it's worked perfectly for me all these years — a well-oiled, well-tuned machine. I'd like for that to continue even if new features are introduced. Addressing these items would demonstrate that you're ready for the job.

After re-considering, I think I'm still not ready to take it over. Maybe in future point. I need to improve my Elisp skill still.