Re: [PATCH 3/3] gnu: Add paprefs

[Leo Famulari]

On Wed, Feb 17, 2016 at 03:02:41PM +0000, Christopher Baines wrote:
> * gnu/packages/pulseaudio.scm (paprefs): New variable

Cool, thanks for the patches! This review is rather superficial since I
don't have a GuixSD desktop system, so I can't really test the
functionality of the software provided by this patch series. I assume it
works for you. Did you test it on GuixSD or on a foreign distro?

> ---
>  gnu/packages/pulseaudio.scm | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)

[...]

> +(define-public paprefs
> +  (package
> +    (name "paprefs")
> +    (version "0.9.10")
> +    (source (origin
> +             (method url-fetch)
> +             (uri (string-append
> +                   
> "https://freedesktop.org/software/pulseaudio/paprefs/paprefs-";
> +                   version
> +                   ".tar.xz"))

I'd prefer if the URL started directly after string-append, and then
used the minimum number of lines, while staying within the 80 character
limit. I can correct this sort of thing when applying patches, but I
must say it out loud once in a while ;)

> +             (sha256
> +              (base32
> +               "1c5b3sb881szavly220q31g7rvpn94wr7ywlk00hqb9zaikml716"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +      '(#:configure-flags '("CXXFLAGS=-std=c++11"))) ; required by gtkmm
> +    (inputs
> +     `(("pulseaudio" ,pulseaudio)
> +       ("gconfmm" ,gconfmm)
> +       ("lynx" ,lynx) ; used for documentation generation

If it's only used to generate the docs, can we make it a native-input,
so it's only available while building?

> +       ("libglademm" ,libglademm)
> +       ("dbus-glib" ,dbus-glib)))
> +    (native-inputs
> +     `(("intltool" ,intltool)
> +       ("pkg-config" ,pkg-config)))
> +    (home-page "https://freedesktop.org/software/pulseaudio/paprefs/";)
> +    (synopsis "simple configuration dialog for PulseAudio")

The linter whines that the synopsis does not start with a capital
letter. What, you don't like the linter? ;)

> +    (description
> +     "PulseAudio Preferences (paprefs) is a simple GTK+ based configuration
> +dialog for the PulseAudio sound server.")
> +    (license l:gpl2+)))
> -- 
> 2.7.0
> 
>