Re: [PATCH v3] Add hplip

[Andy Wingo]

Hi,

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 (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