Subject: Re: [bug#38842] [PATCH 3/3] gnu: Add redsea.

From: Ricardo Wurmus <[email-masked]>
Date: Wed, 01 Jan 2020 23:11:46 +0100

Hi Evan,

> From f19dfe6652727f074b0aa335b43c52bb482116b2 Mon Sep 17 00:00:00 2001
> From: Evan Straw <[email-masked]>
> Date: Tue, 31 Dec 2019 19:04:37 -0800
> Subject: [PATCH 3/3] gnu: Add redsea.
>
> * gnu/packages/ham-radio.scm (redsea): New variable.
> ---
>  gnu/packages/ham-radio.scm | 43 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/gnu/packages/ham-radio.scm b/gnu/packages/ham-radio.scm
> index f327ea22da..5c1e17ac47 100644
> --- a/gnu/packages/ham-radio.scm
> +++ b/gnu/packages/ham-radio.scm
> @@ -20,6 +20,7 @@
>  (define-module (gnu packages ham-radio)
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix packages)
> +  #:use-module (guix utils)

Why is this needed?

> +(define-public redsea
> +  (package
> +    (name "redsea")
> +    (version "0.18")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/windytan/redsea")
> +                    (commit (string-append "v" version))))
> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32
> +                "1y96g0ra2krjb2kypm8s5gdfia45yci4f36klsvyzg8d53v5cwhn"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         ;; The configure.ac file does not explicitly link against libiconv
> +         ;; except on Mac OS, causing the build to fail. This phase comments
> +         ;; out the original AC_SUBST macro (located inside a conditional) and
> +         ;; adds an explicit use of it underneath, so that libiconv is always
> +         ;; linked against.
> +         (add-after 'unpack 'patch-libiconv
> +           (lambda _
> +             (substitute* "configure.ac"
> +               (("^ +AC_SUBST")
> +                "# AC_SUBST")
> +               (("esac")
> +                "esac\nAC_SUBST([ICONV], [\"-liconv\"])")))))))

Please end all phases on #t (or #true).

> +    (inputs `(("libiconv" ,libiconv)
> +              ("libsndfile" ,libsndfile)
> +              ("liquid-dsp" ,liquid-dsp)))
> +    (native-inputs `(("autoconf" ,autoconf)
> +                     ("automake" ,automake)))

I suggest putting the lists on a new line.

> +    (home-page "https://github.com/windytan/redsea")
> +    (synopsis "A lightweight RDS to JSON decoder")

Please remove the leading “A”.  (guix lint should complain about this.)

> +    (description "redsea is a lightweight command-line FM-RDS decoder for
> +GNU/Linux. Redsea can be used with any RTL-SDR USB radio stick with the rtl_fm
> +tool, or any other SDR via csdr, for example. It can also decode raw ASCII
> +bitstream, the hex format used by RDS Spy, and audio files containing
> +multiplex signals (MPX).")

Please remove “for GNU/Linux”.  Please use double spacing.  Could you
please use @dfn{} to define a few of these abbreviations?  The first use
of SDR could be “@dfn{software defined radio} (SDR)” etc.

I would do this for you, but as this depends on liquid-dsp, which needs
some clarifications, could you please also send an updated patch for
this package?

Thank you!

--
Ricardo