[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] Add hplip

From: Andy Wingo
Subject: Re: [PATCH v3] Add hplip
Date: Tue, 22 Mar 2016 11:56:18 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)


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

On Tue 22 Mar 2016 11:30, Danny Milosavljevic <address@hidden> 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 (
> +    ; 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:

            (string-append "--prefix=" ...)

> +                 #: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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]