all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: 62753@debbugs.gnu.org
Cc: d@delgado.nrw
Subject: [bug#62753] [PATCH] gnu: Add emacs-eradio.
Date: Tue, 11 Apr 2023 21:13:32 +0200	[thread overview]
Message-ID: <875ya2nsf7.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <20230410084937.12440-1-d@delgado.nrw> (Dominik Delgado Steuter via Guix-patches via's message of "Mon, 10 Apr 2023 10:49:37 +0200")

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




  reply	other threads:[~2023-04-11 19:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-10  8:49 [bug#62753] [PATCH] gnu: Add emacs-eradio Dominik Delgado Steuter via Guix-patches via
2023-04-11 19:13 ` Nicolas Goaziou [this message]
2023-04-12 14:41   ` Dominik Delgado Steuter via Guix-patches via
2023-04-12 15:16 ` Dominik Delgado Steuter via Guix-patches via
2023-04-16 13:53   ` Nicolas Goaziou
2023-04-16 13:55     ` Nicolas Goaziou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=875ya2nsf7.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=62753@debbugs.gnu.org \
    --cc=d@delgado.nrw \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.