[bug#62753] [PATCH] gnu: Add emacs-eradio.

[Nicolas Goaziou]

Hello,

Dominik Delgado Steuter via Guix-patches via <guix-patches@gnu.org>
writes:

>     * gnu/packages/emacs-xyz.scm (emacs-eradio): New variable.

Thank you for the patch. Some comments follow.

> +(define-public emacs-eradio
> +  (package
> +    (name "emacs-eradio")
> +    (version "20210327.1000")

We don't use MELPA date-based versioning system, unless it is also used
upstream. It isn't the case here as upstream set library's version to
0.1.

> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "https://melpa.org/packages/eradio-";
> +                           version ".el"))

I suggest to use GitHub as upstream.

> +       (sha256
> +        (base32
> +         "0i9mfy5xck0qbbgjhagd35hxd091254w0wga60wd44n04m46h05l"))))
> +    (build-system emacs-build-system)
> +    (home-page "https://github.com/olavfosse/eradio";)
> +    (synopsis "The simple radio player for GNU Emacs")

You should drop "The" in front of the synopsis. I think "./pre-inst-env
guix lint emacs-eradio" should warn about it.

> +    (description
> +     "eradio is a simple Internet radio player for Emacs. Start, stop or
> +toggle custom-defined channels. An external media player like mpv or vlc is
> +required.")

I would capitalize "Eradio", but not "internet". Also I suggest to write
"Mpv" and "VLC".

On this topic, would it make sense to provide VLC as an input, so the
Emacs library works out of the box?

You also need to separate sentences with two spaces, according to
Texinfo syntax.

> +    (license license:gpl3)))

License is actually GPL3+.

Could you send an updated patch?

Regards,
-- 
Nicolas Goaziou