From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: Add GNUMach Headers
Date: Sat, 15 Feb 2014 22:54:02 +0100
Message-ID: <87k3cw11qd.fsf@gnu.org>
References: <CAFtzXzN_MA=P=YT8+hD83HPF=d=T4iMb_eKEr-o5eey5-g+F4g@mail.gmail.com>
	<CAFtzXzOMQ_pMnBY_ufN0c=_KqY_A7X6afc3qUxb5MO6TjJkYGA@mail.gmail.com>
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]:52430)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1WEnBZ-000306-9N
	for Guix-devel@gnu.org; Sat, 15 Feb 2014 16:54:13 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1WEnBV-0000dD-3g
	for Guix-devel@gnu.org; Sat, 15 Feb 2014 16:54:09 -0500
Received: from hera.aquilenet.fr ([2a01:474::1]:47037)
	by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <ludo@gnu.org>)
	id 1WEnBU-0000d9-Ss
	for Guix-devel@gnu.org; Sat, 15 Feb 2014 16:54:05 -0500
In-Reply-To: <CAFtzXzOMQ_pMnBY_ufN0c=_KqY_A7X6afc3qUxb5MO6TjJkYGA@mail.gmail.com>
	(=?utf-8?B?Is6czpHOnc6pzpvOl86jIM6hzpHOk86azp/Opc6jzpfOoyIncw==?=
	message of "Sat, 15 Feb 2014 21:28:19 +0000")
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: =?utf-8?B?zpzOkc6dzqnOm86XzqMgzqHOkc6TzprOn86lzqPOl86j?= <manolis837@gmail.com>
Cc: Guix-devel@gnu.org

Hello!

Thanks for the patch, and welcome!

This looks good to me, but as a welcome present ;-), I=E2=80=99ll do some
nitpicking so you can learn about the conventions in use.

=CE=9C=CE=91=CE=9D=CE=A9=CE=9B=CE=97=CE=A3 =CE=A1=CE=91=CE=93=CE=9A=CE=9F=
=CE=A5=CE=A3=CE=97=CE=A3 <manolis837@gmail.com> skribis:

> From ff3ee75b23e33b0250ce64ba90969d2a8ad4c521 Mon Sep 17 00:00:00 2001
> From: Manolis Ragkousis <manolis837@gmail.com>
> Date: Sat, 15 Feb 2014 22:05:29 +0000
> Subject: [PATCH] Add GNU Mach kernel headers
>
> ---
>  gnu-system.am         |  1 +
>  gnu/packages/hurd.scm | 54 +++++++++++++++++++++++++++++++++++++++++++++=
++++++

Please add a ChangeLog-style commit log entry (see the =E2=80=98HACKING=E2=
=80=99 file
for details.)

Also, we usually prefix with =E2=80=98gnu: =E2=80=99 the subject line when =
changing
something under the gnu/ directory.

In general, see =E2=80=98git log=E2=80=99 when in doubt.  :-)

> +    (source
> +     (origin
> +      (method url-fetch)
> +      (uri (string-append "http://ftp.gnu.org.ua/gnu/gnumach/gnumach-"
> +                          version ".tar.gz"))

This should be =E2=80=9Cmirror://gnu/gnumach/...=E2=80=9D to allow the use =
of gnu.org
mirrors.

> +    (arguments=20
> +     `( #:phases (alist-replace
          ^
There should be no space here.

> +                  'install=20
> +                  (lambda _ (zero?
> +                             (system* "make" "install-data")))

Rather:

  (lambda _ ;; <- always skip a line after the lambda argument list
    (zero? ...))

> +                  (alist-delete
> +                   'build
> +                   %standard-phases))=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20
> +        #:tests? #f ))
                      ^
No space here.

> +;;TODO: package the rest of the components of the GNU Hurd system

No need to put it here, this is implicit.  :-)

Could you send an updated patch?

Thanks for diving into this!

Ludo=E2=80=99.