Re: [PATCH] gnu: Add wmbattery

[Alex Kost]

Kei Yamashita (2016-04-05 00:04 +0300) wrote:

> The source tarball comes from Debian. Hopefully this patch is formatted
> correctly.
>
> From ed508727588e234a89e1a34920d29d118c848509 Mon Sep 17 00:00:00 2001
> From: Kei Yamashita <address@hidden>
> Date: Mon, 4 Apr 2016 16:59:02 -0400
> Subject: [PATCH] gnu: Add wmbattery
                                     ^
Conventions everywhere: a period in the end of the commit message :-)

> * gnu/packages/gnustep.scm (wmbattery): New variable.
> ---
>  gnu/packages/gnustep.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/gnu/packages/gnustep.scm b/gnu/packages/gnustep.scm
> index edd159d..a5722a2 100644
> --- a/gnu/packages/gnustep.scm
> +++ b/gnu/packages/gnustep.scm
> @@ -22,6 +22,10 @@
>    #:use-module (guix build-system gnu)
>    #:use-module (guix licenses)
>    #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages gnome)
> +  #:use-module (gnu packages texinfo)
> +  #:use-module (gnu packages autotools)
> +  #:use-module (gnu packages glib)
>    #:use-module (gnu packages fontutils)
>    #:use-module (gnu packages image)
>    #:use-module (gnu packages pkg-config))
> @@ -83,3 +87,42 @@ interface.  It is fast, feature rich, easy to configure, 
> and easy to use.")
>  
>      ;; Artwork is distributed under the WTFPL.
>      (license gpl2+)))
> +
> +(define-public wmbattery
> +  (package
> +    (name "wmbattery")
> +    (version "2.50")
> +    (source (origin
> +           (method url-fetch)
   ^^^^
As Danny pointed we don't use tabulations.

> +           (uri (string-append
> +                 "http://http.debian.net/debian/pool/main/w/wmbattery/\
> +wmbattery_2.50.orig.tar.gz"))

We don't split strings like this; with 'string-append' you can make as
many substrings as needed.  Also I changed it to use "mirror://debian/…".

> +           (sha256
> +            (base32
> +             "0hi6bivv3xd2k68w08krndfl68wdx7nmc2wjzsmcd4q3qgwgyk44"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:phases (modify-phases %standard-phases
> +                  (add-before
> +                      'configure 'autoconf

It looks better when the phases names are moved to the previous line.

Also this autoreconf phase should be added after 'unpack', not before
'configure', because between 'unpack' and 'configure' there are other
phases that fix potential problem on other systems (mips and arm).

> +                    (lambda _ (zero? (system* "autoreconf" "-vfi"))))
> +                  (delete 'check))))

Instead of deleting "check" phase, we disable tests explaining the reason.

I made the mentioned changes and applied your patch as f9ab1da¹.  Thanks!

¹ 
http://git.savannah.gnu.org/cgit/guix.git/commit/?id=f9ab1da0fa1a4cdb100209c2607c7afbbacfb7c6

-- 
Alex