From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andy Wingo <wingo@igalia.com>
Subject: Re: [PATCH v3] Add hplip
Date: Tue, 22 Mar 2016 11:56:18 +0100
Message-ID: <87egb2u6gd.fsf@igalia.com>
References: <20160312191233.48a661ca@scratchpost.org> <8737ruuj0l.fsf@gnu.org>
	<20160315210512.23989245@scratchpost.org>
	<20160315221931.16abe3dc@scratchpost.org> <87vb4mygw4.fsf@igalia.com>
	<87mvpyyfon.fsf@igalia.com> <20160320011904.50131806@scratchpost.org>
	<20160321230700.452ca6c1@scratchpost.org> <87r3f2ud9x.fsf@igalia.com>
	<20160322113001.52af072b@scratchpost.org>
Mime-Version: 1.0
Content-Type: text/plain
Return-path: <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58107)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <wingo@igalia.com>) id 1aiJzA-00060z-7u
	for guix-devel@gnu.org; Tue, 22 Mar 2016 06:56:29 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <wingo@igalia.com>) id 1aiJz5-00082v-5V
	for guix-devel@gnu.org; Tue, 22 Mar 2016 06:56:28 -0400
Received: from pb-sasl0.pobox.com ([208.72.237.25]:65457
	helo=sasl.smtp.pobox.com) by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <wingo@igalia.com>) id 1aiJz4-00082O-VY
	for guix-devel@gnu.org; Tue, 22 Mar 2016 06:56:23 -0400
In-Reply-To: <20160322113001.52af072b@scratchpost.org> (Danny Milosavljevic's
	message of "Tue, 22 Mar 2016 11:30:01 +0100")
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: Danny Milosavljevic <dannym@scratchpost.org>
Cc: guix-devel@gnu.org

Hi,

Looking better, thank you!  A couple of style nits and a tip.

On Tue 22 Mar 2016 11:30, Danny Milosavljevic <dannym@scratchpost.org> writes:

> +    (description "Hewlett-Packard Printer Drivers and PPDs.")
> +    (license (list license:gpl2 license:bsd-3)) ; FIXME and which MIT
> +    ; FIXME remove proprietary plug-in installer, hp-plugin (plugin.py)
> +    ; FIXME PPDs use "hpcups" in cupsFilter. In which directory?
> +    ; TODO install apparmor profile files

Comments that align with the expression on the next line should start
with ";;".  This kind of comment should be full sentences.  Good to
leave in a note about AppArmor I guess, though it's not a current thing
in Guix.  See other packages for examples of how to delete files.

> +    (arguments `(#:configure-flags `("--disable-network-build"
> +                                     ,(string-append "--prefix="

This sort of thing is more usually indented as:

       (arguments
        `(#:configure-flags
          `("--disable-network-build"
            (string-append "--prefix=" ...)
            ...)
          #:phases
          ...))

> +                 #:phases (alist-cons-after 'fix-libusb 'autoreconf

Please use modify-phases instead of alist-cons-after.

> +              ; I don't think we need to propagate those.
> +              ; wrapper: makes "python" available rather than "python3"

No need to leave in these comments, IMO.

> +              ("python-wrapper" ,python-wrapper)
> +              ("python" ,python) ; patch-shebang still complains
> +              ; TODO: make hp-setup find python-dbus
> +              ("python-dbus" ,python-dbus)))
> +    ;(propagated-inputs `(("python" ,python)
> +    ;                     ("python-dbus" ,python-dbus)))

You can probably can remove this too.

Cheers,

Andy