From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: [PATCH] Add gerbv.
Date: Tue, 01 Sep 2015 23:14:57 +0200
Message-ID: <87r3mhet9q.fsf@gnu.org>
References: <87y4gsgddl.fsf@elephly.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Return-path: <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:50850)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1ZWstU-00069M-Br
	for guix-devel@gnu.org; Tue, 01 Sep 2015 17:15:05 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1ZWstR-0006RG-6U
	for guix-devel@gnu.org; Tue, 01 Sep 2015 17:15:04 -0400
In-Reply-To: <87y4gsgddl.fsf@elephly.net> (Ricardo Wurmus's message of "Mon,
	31 Aug 2015 08:50:46 +0200")
List-Id: "Development of GNU Guix and the GNU System distribution."
	<guix-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-devel>
List-Post: <mailto:guix-devel@gnu.org>
List-Help: <mailto:guix-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=subscribe>
Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org
Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org
To: Ricardo Wurmus <rekado@elephly.net>
Cc: guix-devel <guix-devel@gnu.org>

Ricardo Wurmus <rekado@elephly.net> skribis:

> From e992f3d52f99219f25f00d1917ca6f83f9e805f3 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Mon, 31 Aug 2015 08:46:12 +0200
> Subject: [PATCH 1/2] gnu: Add po4a.
>
> * gnu/packages/gettext.scm (po4a): New variable.

[...]

> +            (substitute* "Po4aBuilder.pm"
> +              ;; By default it tries to install into perl's manpath.
> +              (("my \\$mandir =3D .*$")
> +               (string-append "my $mandir =3D \"" (assoc-ref outputs "ou=
t")
> +                              "/share/man\";\n"))
> +              ;; Use local docbook XSL
> +              (((string-append "--nonet http://docbook.sourceforge.net/"
> +                                "release/xsl/current"
> +                                "/manpages/docbook.xsl"))

Rather make it a string literal, using backslashes for newlines, but...

> +               (string-append "--nonet "
> +                              (assoc-ref inputs "docbook-xsl")
> +                              ;; TODO: replace version string
> +                              "/xml/xsl/docbook-xsl-"  "1.78.1"
> +                              "/manpages/docbook.xsl")))

... can this be avoided altogether by adding libxml2 as an input (which
sets $XML_CATALOG_FILES as a side-effect)?

Otherwise LGTM.

> From 87f3d5df4cb7672e6dcdaa1e4e2b7fc59ff37e11 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Mon, 31 Aug 2015 08:47:09 +0200
> Subject: [PATCH 2/2] gnu: Add gerbv.
>
> * gnu/packages/engineering.scm (gerbv): New variable.

[...]

> +            ;; Build rules contain references to Russian translation, bu=
t the
> +            ;; needed files are missing; see
> +            ;; http://sourceforge.net/p/gerbv/bugs/174/
> +            (delete-file "po/LINGUAS")
> +            (substitute* "man/Makefile.am"
> +              (("PO_FILES=3D gerbv.ru.1.in.po") "")
> +              (("man_MANS =3D gerbv.1 gerbv.ru.1") "man_MANS =3D gerbv.1=
"))
> +            (zero? (system* "autoreconf" "-vfi")))))))

Could you modify Makefile.in instead?  That would hopefully allow us to
remove the dependency on the autotools.

> +     "Gerbv is a Gerber file (RS-274X only) viewer.  Gerbv lets you load

What about giving a bit more context, along the lines of:

  Gerbv is a viewer for files in the Gerber format (RS-274X only), which
  is commonly used to represent printed circuit board (PCB) layouts.

?

Otherwise, LGTM.

Thanks,
Ludo=E2=80=99.