Subject: Re: [bug#52875] [PATCH] gnu: Add musikcube.

From: Nicolas Goaziou <[email-masked]>
Date: Thu, 30 Dec 2021 00:13:49 +0100

Hello,

Leo Famulari <[email-masked]> writes:

> * gnu/packages/music.scm (musikcube): New variable.

Nice! Thanks.

> +(define-public musikcube
> +  (package
> +    (name "musikcube")
> +    (version "0.96.10")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                     (url "https://github.com/clangen/musikcube/")
> +                     (commit version)))
> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32
> +                "06myr83x8jvzlzchg3jsw1163n2lcsbmb176zgnx7xxa26jpdbh1"))))

Nitpick: origin could go below source, and hash in front of base32.

> +    (build-system cmake-build-system)
> +    (arguments
> +     (list
> +       #:tests? #f ; No test suite

Note that indentation is a bit off.

> +       #:configure-flags
> +       #~(list
> +           "-DCMAKE_BUILD_TYPE=Release"
> +           "-DENABLE_BUNDLED_TAGLIB=false"
> +           ;; Use the "wide" ncurses headers but don't look for them in an
> +           ;; ncursesw directory. For more info:
> +           ;; https://github.com/clangen/musikcube/wiki/building#compiler-cannot-find-ncurseswcursesh
> +           "-DNO_NCURSESW=true"
> +           ;; We will strip the binaries ourselves in the 'strip' phase.
> +           "-DDISABLE_STRIP=true")))

OOC, why are use using gexp since you do not ungexp later on? Wouldn't
a regular quote, or even a quasiquote suffice?

Otherwise, LGTM.

Regards,
-- 
Nicolas Goaziou