Re: [PATCH 1/1] gnu: add feh

[Alex Kost <alezost@gmail.com>]

Hello,

I confirm that the package is built successfully (at least for me).

Several comments:

There should be a header (a comment with author, license, …) in the
beginning of “feh.scm” – see any file in the "gnu/packages" dir.

Ian Denhardt (2014-10-05 07:32 +0400) wrote:

[...]

> +(define-public feh
> +  (package
> +    (name "feh")
> +    (version "2.12")
> +    (home-page "https://feh.finalrewind.org/")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append home-page
> +                                  name "-" version  ".tar.bz2"))

Double space after ‘version’.

> +              (sha256 (base32
> +                "0ckhidmsms2l5jycp0qf71jzmb3bpbhjq3bcgfpvfvszah7pmq30"))))

This is rather unusual indentation.  As for me, I prefer:

(sha256
 (base32
  "0ckhidmsms2l5jycp0qf71jzmb3bpbhjq3bcgfpvfvszah7pmq30"))

People also use:

(sha256 (base32
         "0ckhidmsms2l5jycp0qf71jzmb3bpbhjq3bcgfpvfvszah7pmq30"))

> +    (build-system gnu-build-system)
> +    (arguments 

Remove trailing space here please.

> +      '(#:phases (alist-delete 'configure %standard-phases)
> +        #:tests? #f
> +        #:make-flags
> +          (list "CC=gcc" (string-append "PREFIX=" (assoc-ref %outputs "out")))))
> +    (inputs `(("imlib2" ,imlib2)
> +              ("curl" ,curl)
> +              ("libpng" ,libpng)
> +              ("libxt" ,libxt)
> +              ("libx11" ,libx11)
> +              ("libxinerama" ,libxinerama)))
> +    (synopsis "Fast and light imlib2-based image viewer")
> +    (description (string-append
> +                   "feh is an X11 image viewer aimed mostly at console users. "
> +                   "Unlike most other viewers, it does not have a fancy GUI, "
> +                   "but simply displays images. It is controlled via "
> +                   "commandline arguments and configurable key/mouse actions."))

This will make a long one-line description, which is not preferable.
Descriptions should be multi-lined and they usually look like this:

--8<---------------cut here---------------start------------->8---
    (description
     "feh is an X11 image viewer aimed mostly at console users.
Unlike most other viewers, it does not have a fancy GUI, but simply
displays images.  It is controlled via commandline arguments and
configurable key/mouse actions.")
--8<---------------cut here---------------end--------------->8---

Also I think it is worth mentioning (in the description) that feh can
also be used as a wallpaper setter.

-- 
Thanks,
Alex