Skip to content
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

Add to ELPA/MELPA? #3

Open
Ambrevar opened this issue Aug 14, 2018 · 5 comments
Open

Add to ELPA/MELPA? #3

Ambrevar opened this issue Aug 14, 2018 · 5 comments

Comments

@Ambrevar
Copy link

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

@skeeto
Copy link
Owner

skeeto commented Aug 14, 2018 via email

@Ambrevar
Copy link
Author

Ambrevar commented Aug 14, 2018 via email

@stardiviner
Copy link

stardiviner commented Aug 2, 2021

@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
Copy link
Owner

skeeto commented Aug 9, 2021 via email

@stardiviner
Copy link

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

  • 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

  • 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants