diff mbox series

[bug#55769] gnu: Add xwhite.

Message ID TYCP286MB1123CECB77358C7315CAE47ABEDE9@TYCP286MB1123.JPNP286.PROD.OUTLOOK.COM
State Accepted
Headers show
Series [bug#55769] gnu: Add xwhite. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

derekchuank@outlook.com June 2, 2022, 3:39 p.m. UTC

Comments

M June 2, 2022, 6:43 p.m. UTC | #1
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
diff mbox series

Patch

diff --git a/gnu/packages/xdisorg.scm b/gnu/packages/xdisorg.scm
index e5a98edb35..250860274d 100644
--- a/gnu/packages/xdisorg.scm
+++ b/gnu/packages/xdisorg.scm
@@ -54,6 +54,7 @@ 
 ;;; Copyright © 2021 jgart <jgart@dismail.de>
 ;;; Copyright © 2022 John Kehayias <john.kehayias@protonmail.com>
 ;;; Copyright © 2022 Jai Vetrivelan <jaivetrivelan@gmail.com>
+;;; Copyright © 2022 Derek Chuank <derekchuank@outlook.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1499,6 +1500,40 @@  (define-public redshift-wayland
 protocol.")
       (license license:gpl3+))))

+(define-public xwhite
+  (package
+    (name "xwhite")
+    (version "0.0.1")
+    (source
+     (origin
+       (method url-fetch)
+       (uri
+        (string-append "https://github.com/derekchuank/xwhite/"
+                       "releases/download/v" version
+                       "/xwhite-" version ".tar.gz"))
+       (sha256
+        (base32 "1gadgvl408zdl9drz6rb0dhq285a6w57bn94ql2sgcpp9mq7ym5q"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f
+       #: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))))))
+    (inputs
+     (list libxrandr))
+    (home-page "https://github.com/derekchuank/xwhite")
+    (synopsis "Adjust color balance")
+    (description "xwhite is a command line tool for adjusting color
+balance of screen.  It is based on xrandr's gamma correction and
+brightness adjustment.  Typically used for tuning color balance
+while setting color temperature.")
+    (license license:gpl2)))
+
 (define-public gammastep
   (package
     (name "gammastep")