Subject: [bug#71990] [PATCH] gnu: Add emacs-empv.

From: Nicolas Goaziou <[email-masked]>
Date: Mon, 08 Jul 2024 10:02:05 +0200

Hello,

Thanos Apollo <[email-masked]> writes:

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

Thank you. Some comments follow.

> +(define-public emacs-empv
> +  (let ((commit "631299ae2a918b4af17ae94eddcde4c9451235bb"))
> +    (package
> +      (name "emacs-empv")
> +      (version "v4.2.0")

Upstream version is confusing. The main file says its 4.3.0, but last
tag, created one day before bumping to 4.3.0, mentions 4.2.0-RC1.

Moreover, commit 631299... does not match a version bump. So, you should
also bind `revision' to, e.g., "0". and use `git-file-name'.

Eventually, you need to remove the "v" from the version string. It
belongs to the tag, not the version.

> +      (source
> +       (origin
> +         (method git-fetch)
> +         (uri (git-reference
> +               (url "https://github.com/isamert/empv.el")
> +               (commit commit)))
> +         (file-name (git-file-name name version))
> +         (sha256
> +          (base32
> +           "0njqzri2qvpsdbkrkr1lidg2zc2742nzapwgdjaf5ms3rl8kc898"))))
> +      (build-system emacs-build-system)
> +      (propagated-inputs
> +       (list emacs-compat
> +	     emacs-s))

You can write propagated inputs on a single line.

  (propagated-inputs (list emacs-compat emacs-s))

> +      (home-page "https://github.com/isamert/empv.el")
> +      (synopsis "An Emacs media player, media library manager, radio player, YouTube frontend ")

Please remove the article "An" and the trailing white space.

> +      (description
> +       "An Emacs media player, based on mpv.  More precisely this package
> +provides somewhat comprehensive interface to mpv with bunch of
> +convenient functionality like an embedded radio manager, YouTube
> +interface, local music/video library manager etc.")
> +      (license license:gpl3+))))

Description should consist of complete sentences. I suggest something
like:

  "This package is an Emacs player, based on MPV.  More precisely, it
  [...], local music and video library manager, etc."

Could you send an updated patch?

Regards,
-- 
Nicolas Goaziou