Re: [PATCH] xscreenshot and imagefile

[Alex Kost]

Orchidaceae Phalaenopsis (2015-07-27 20:50 +0300) wrote:

> Hello
>
> I would like to contribute packages for two programs:
>
> * xscreenshot which takes a screenshot and saves it as .if format
> * imagefile which contains a few programs to convert .if to gif, png

Hello, thanks for contributing!

> From ee624abf3aada75b5fa234eec0b28fd5e2c561b0 Mon Sep 17 00:00:00 2001
> From: Alf Levan <address@hidden>
> Date: Mon, 27 Jul 2015 18:34:44 +0100
> Subject: [PATCH 1/2] gnu: Add xscreenshot.
>
> * gnu/packages/suckless.scm (xscreenshot): New variable.

Please add a copyright line for yourself to "suckless.scm" in the first
patch:

;;; Copyright © 2015 Alf Levan <address@hidden>

BTW is it "Alf Levan" (as in the commits) or "Orchidaceae Phalaenopsis"
(as in the emails)?

[...]
> +(define-public xscreenshot
> +  (package
> +    (name "xscreenshot")
> +    (version "1.0")
> +    (source (origin
> +          (method git-fetch)
> +          (uri (git-reference
> +                (url "git://git.2f30.org/xscreenshot")
> +                (commit "cb8ad23")))

I think it is better to use "1.0.cb8ad23" for version, so:

(let ((commit "cb8ad23"))
  (package ...))

See 'guix-devel' package, for example.  The same for 'imagefile'
package.

> +          (sha256
> +           (base32
> +            "1sqhxybzi8mmb0jg78swp0vpbdiqhckd283k33lfffsd0lkgch88"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     '(#:tests? #f ; no tests
> +       #:make-flags (list "CC=gcc"
> +                          (string-append "PREFIX=" %output))
> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure))))
> +    (inputs
> +     `(("libx11" ,libx11)))
> +    (native-inputs `(("pkg-config" ,pkg-config)))
> +    (home-page "http://git.2f30.org/xscreenshot/";)
> +    (synopsis "Make screenshot")
> +    (description
> +     "xscreenshot is a simple screenshot utility. It writes image data to 
> stdout.")

Description lines for both packages are too long, please fill to 72-78
column.

> +    (license license:x11)))

AFAICS <http://git.2f30.org/xscreenshot/plain/LICENSE> is the same as
<http://directory.fsf.org/wiki/License:Expat>, so it should be 'expat',
not 'x11'.

[...]
> +(define-public imagefile
> +  (package
> +    (name "imagefile")
> +    (version "1.0")

This thing doesn't mention any version in the source code at all, so I
don't know, maybe just use "0.787ebb0" (with the used commit).  What do
others think?

-- 
Alex