Hi,

guil_fsf--- via Guix-patches via schreef op vr 30-07-2021 om 15:30 [+0200]:
> ---
> gnu/packages/mpd.scm | 50 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)

A commit message is missing.

> diff --git a/gnu/packages/mpd.scm b/gnu/packages/mpd.scm
> index b97abcafc3..2c1f189421 100644
> --- a/gnu/packages/mpd.scm
> +++ b/gnu/packages/mpd.scm
> @@ -513,3 +513,53 @@ for your favorite MPD client but an addition to get a better
> album-experience.")
>      (home-page "https://gitlab.com/coderkun/mcg")
>      (license license:gpl3+)))

> +
> +(define-module (vimpc)
> +  #:use-module (guix licenses)
> +  #:use-module (guix packages)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (guix git-download)
> +  #:use-module (gnu packages mpd)
> +  #:use-module (gnu packages pcre)
> +  #:use-module (gnu packages mp3)
> +  #:use-module (gnu packages autotools)
> +  #:use-module (gnu packages curl)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages ncurses))

This define-module block can be removed,
and the #:use-module can be moved to the top of the file.

> +(define-public vimpc
> +  (package
> +   (name "vimpc")
> +   (version "0.09.2")
> +
> +   (source (origin
> +            (method git-fetch)
> +            (uri (git-reference
> +                  (url "https://github.com/boysetsfrog/vimpc.git")

I think there is a linter checking that ".git" is not included in GitHub
URLs.  Try "./pre-inst-env guix lint vimpc".

> +                  (commit (string-append "v" version))))
> +            (sha256 (base32 "0lswzkap2nm7v5h7ppb6a64cb35rajysd09nb204rxgrkij4m6nx"))
> +            (file-name (git-file-name name version))))
> +   (build-system gnu-build-system)
> +   (native-inputs `(("autoconf" ,autoconf)
> +     ("automake" ,automake)))
> +   (inputs `(("libmpdclient" ,libmpdclient)
> +      ("pcre" ,pcre)
> +      ("taglib" ,taglib)
> +      ("pkg-config" ,pkg-config)

"pkg-config" should most likely be in 'native-inputs', not 'inputs'.
The difference between 'inputs' and 'native-inputs' matters when
cross-compiling.

I'd suggest running
"./pre-inst-env guix build vimpc --target=x86_64-linux-gnu"
for testing, but meson-build-system (used by libmpdclient) doesn't
support cross-compilation yet (at least, on master it doesn't.  On the
core-updates branch, meson-build-system supports cross-compilation),
so that wouldn't work yet anyway.

> +      ("curl" ,curl)
> +      ("ncurses" ,ncurses)
> +      ))
> +   (arguments
> +       `(#:tests? #false

When disabling tests, always write a comment explaining why.
If the tests fail, that may indicate a real problem (that's
the purpose of tests).

> +
> +         #:phases
> +         (modify-phases %standard-phases
> +           (add-after 'unpack 'fix-hardcoded-paths
> +             (lambda _
> +                  (system* "./autogen")
> +        )))))

The formatting went wrong here (whitespace and lonely parentheses).
I'd recommend using "git format-patch HEAD^..HEAD" and sending the patch
as an attachment, or using "git send-email".

> +   (synopsis "Vi-like Mpd TUI client written in C++")
> +   (description "Vimpc provides an alternative to other mpd clients (such as ncmpc and ncmpcpp) that tries to provide an interface similar to that of the vim text editor.")

This description doesn't explain anything to people who don't know what
'Mpd', 'Vi', 'nmpc' and 'ncmpcpp' are.  I only know of the first two
(a music/video player and a customisable text editor), but only because
I've read something about it some time ago. 

Browsing through the source code, I notice files named "lyricsloader.cpp",
"lyricsfetcher.cpp", "song.cpp".  Maybe you could note that "vimpc"
is a music player?

Greetings,
Maxime.