Re: gnu: Add weechat

[Eric Bavier]

Just some stylistic comments:

address@hidden writes:

> From 8d8d1e052623d052834c84cbf433d5ca95c6e513 Mon Sep 17 00:00:00 2001
> From: Kevin Lemonnier <address@hidden>
> Date: Fri, 12 Sep 2014 00:32:20 +0200
> Subject: [PATCH] gnu: Add weechat
>
> * gnu/packages/weechat.scm: New file

Period at the end.

> diff --git a/gnu/packages/weechat.scm b/gnu/packages/weechat.scm
> new file mode 100644
> index 0000000..2391017
> --- /dev/null
> +++ b/gnu/packages/weechat.scm
> @@ -0,0 +1,102 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright 2014 Kevin Lemonnier <address@hidden>

You might also include the <copyright-sign> that is present in other
source headers.

> +(define-module (gnu packages weechat)
> +  #:use-module (gnu packages ncurses)
> +  #:use-module (gnu packages base)
> +  #:use-module (gnu packages gettext)
> +  #:use-module (gnu packages gnupg)
> +  #:use-module (gnu packages file)
> +  #:use-module (gnu packages compression)
> +  #:use-module (gnu packages lua)
> +;  #:use-module (gnu packages ruby)

Rather than commenting-out the #:use-module and the input, could we
instead leave a TODO comment to add ruby at a later date?

> +(define-public weechat
> +  (package
> +    (name "weechat")
> +    (version "1.0")
> +    (source (origin
> +             (method url-fetch)
> +             (uri (string-append "http://weechat.org/files/src/weechat-";
> +                                  version ".tar.gz"))
> +             (sha256
> +              (base32 
> "1z17wyrl5fp697qp44srpmzk79w37f5hm1r0krffbmga6sbzdj3x"))))
> +    (build-system gnu-build-system)
> +    (native-inputs `(("autoconf", autoconf)

Put the unquote ',' directly before the symbol that is being unquoted.

> +                     ("pkg-config", pkg-config)))
> +    (inputs `(("ncurses", ncurses)
> +              ("diffutils", diffutils)
> +              ("gettext", gnu-gettext)
> +              ("automake", automake)
> +              ("libtool", libtool "bin")
> +              ("libtool", libtool "out")
> +              ("libgcrypt", libgcrypt "out")
> +              ("file", file)

I'm guessing that 'file' should be a native-input.  Is it run during the
build, or also at runtime?

> +              ("zlib", zlib)
> +              ("aspell", aspell)
> +              ("curl", curl)
> +              ("gnutls", gnutls)
> +              ("guile", guile-2.0)
> +              ("openssl", openssl)
> +              ("cyrus-sasl", cyrus-sasl)
> +              ("lua", lua-5.1)
> +;              ("ruby", ruby)

See comment above.

> +              ("python", python-2)
> +              ("perl", perl)
> +              ("tcl", tcl)
> +              ("autogen", autogen)))
> +    (arguments `(#:configure-flags (list
> +                                    (string-append
> +                                     "--with-tclconfig="
> +                                     (assoc-ref %build-inputs "tcl") "/lib"))
> +                 #:phases (alist-cons-after
> +                           'autogen 'fix-file
> +                           (lambda _
> +                             (substitute*
> +                                 '("configure")

Put "configure" on the same line as substitute*.  If it's a single file,
substitute* will accept it on its own, i.e. (substitute* "configure" ...).

> +                               (("/usr/bin/file") (which "file"))))
> +                           (alist-cons-before
> +                            'configure 'autogen
> +                            (lambda _          
> +                              (zero? (system* "./autogen.sh")))
> +                            %standard-phases))))
> +    (synopsis "Fast, light and extensible ncurses chat client")
> +    (description "WeeChat (Wee Enhanced Environment for Chat) is a free
> +and open-source Internet Relay Chat client, which is designed to be

Leave out the "free and open-source" bit since, as Ludo would say, we're
already in the land of freedom. ;)

Thanks!

-- 
Eric Bavier

Please avoid sending me Word or PowerPoint attachments.
See http://www.gnu.org/philosophy/no-word-attachments.html