[bug#55769] [PATCH] gnu: Add xwhite.

[Maxime Devos]

derekchuank@outlook.com schreef op do 02-06-2022 om 15:39 [+0000]:
> +    (arguments
> +     `(#:tests? #f

Always add a comment on why tests are disabled.
In this case, probably because there are no tests?

> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let ((out (assoc-ref outputs "out")))
> +               (install-file "xwhite" (string-append out "/bin"))
> +               #t))))))

Trailing #t haven't been necessary anymore since a long time

Also, a comment on the makefile.  In the makefile you do CC=gcc, but
that won't work when cross-compiling, because then TARGET-gcc (e.g. 
aarch64-linux-gnu-gcc) is required.  There are multiple ways to resolve
this, but given that you are upstream, I recommend switching to a build
system such as, say, meson[0].  meson (and autotools) take care of such
details for you (cross-compilation, also choosing the right
installation directory).

If you use meson or autotools, then you won't have to add any Guix
phases or do cross-compilation detection in the makefile, you then only
have to replace gnu-build-system by
meson-build-system.

A comment on the license: strictly speaking, you have licensed it as
‘any version of the GPL whatsoever’.  From the GPL itself:

> If the Program does not specify a version number of this License, you
> may choose any version ever published by the Free Software
> Foundation.

I doubt that's what you mean, so maybe document it in the README to be
GPL-2-or-later or GPL-2-only or GPL-2-OR-3-only?

A comment on the code itself: I recommend returning a non-zero value on
failure.  Also, all this is already implemented by another tool:

$ redshift -P -g 2:2:0.1 -O 10000

so to me there appears to be no need to write, maintain and package a
new tool.

[0] https://mesonbuild.com/

Greetings,
Maxime

signature.asc
Description: This is a digitally signed message part