Re: [PATCH] Added package xcalib

[Leo Famulari <leo@famulari.name>]

On Mon, Nov 14, 2016 at 05:05:55PM +0100, Petter wrote:
> Hi,
> 
> I just made a recipe for xcalib, with help from iyzsong. This is the first
> time I've created one so please be extra alert while reviewing, and don't
> hesitate to inform me of mistakes.

Thanks Petter and iyzsong!

I think the package should go in the (gnu packages xdisorg) module
instead of its own module.

> From 00ca66ec205baf52dc340faa61fd8ac38122e273 Mon Sep 17 00:00:00 2001
> From: Petter <petter@mykolab.ch>
> Date: Mon, 14 Nov 2016 16:56:33 +0100
> Subject: [PATCH] Added package xcalib

The commit title should be "gnu: Add xcalib.".

Also, please add a "changelog" line describing the code changes. In this
case, it should be:

* gnu/packages/xdisorg (xcalib): New variable.

This is the GNU convention, and we mention it in the manual, section 8.5
Submitting Patches [0]. Please refer to earlier Git commits or ask for help
on IRC if you are unsure about it.

[0]
https://www.gnu.org/software/guix/manual/html_node/Submitting-Patches.html

> +(define-public xcalib
> +  (package
> +    (name "xcalib")
> +    (version "0.8")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "http://downloads.sourceforge.net/xcalib/"
> +                                  "xcalib-source-" version ".tar.gz"))

We have a SourceForge "mirror" URL that should be used. Grep for
'mirror://sourceforge' in gnu/packages for some examples of how to use
it.

> +    (arguments
> +     '(#:make-flags '("CC=gcc")
> +       #:phases (modify-phases %standard-phases
> +                  (delete 'configure)
> +                  (delete 'check)

Please add a comment explaining why we skip the tests. If there is no
test suite, just use "No test suite". Also, we prefer to skip tests by
setting #:tests? #f instead of deleting the check phase.

> +                  (replace 'install
> +                    (lambda* (#:key outputs #:allow-other-keys)
> +                      (let* ((out (assoc-ref outputs "out"))
> +                             (bin (string-append out "/bin")))
> +                        (mkdir-p bin)
> +                        (install-file "xcalib" bin)))))))

Okay. Please check if there is any documentation that could also be
installed. You can add a build phase before 'install that just fails:
(lambda _ #f), build with --keep-failed, and then look in the build
directory for man pages, READMEs, Info pages, etc.

> +    (license gpl2)))

Please double-check if the source files contain the "or later" text in
the license headers. If so, it's gpl2+.