guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#34572] Add Drawpile


From: Tobias Geerinckx-Rice
Subject: [bug#34572] Add Drawpile
Date: Thu, 21 Feb 2019 14:58:33 +0100

Pkill -9,

Thanks for this patch, and your many others.

pkill9 wrote:
+  #:use-module (gnu packages crypto) ; libsodium
+  #:use-module (gnu packages gnunet) ; libmicrohttpd

In my experience, what little value such comments add is quickly lost. Anyone adding new inputs will not update (or even notice) them.

+       ("giflib" ,giflib) ; optional
+       ("kdnssd" ,kdnssd) ; optional
+       ("miniupnpc" ,miniupnpc) ; optional
+       ("libmicrohttpd" ,libmicrohttpd) ; optional
+       ("libsodium" ,libsodium))) ; optional

Same here: nothing wrong with these, I guess, but *many* package dependencies are optionally detected at build time and this isn't usually pointed out unless there's something more interesting going on.

+    (arguments
+     `(#:configure-flags
+       (list "-DTESTS=on" ; build unit tests.

General remark: no full stop after inline comments.

  ;; Foo bar.
  (foo bar) ; foo bar

+             "-DTOOLS=on" ; build dprec2txt command line tool.
+             (string-append "-DLIBQTCOLORWIDGETS_LIBRARY="
+ (assoc-ref %build-inputs "qtcolorwidgets") + "/lib/libQtColorWidgets-Qt52.so"))))

What about using FIND-FILES "\*.so$" here instead of hard-coding "52"? Overkill?

Kind regards,

T G-R





reply via email to

[Prev in Thread] Current Thread [Next in Thread]