Subject: [bug#34572] Add Drawpile

From: Tobias Geerinckx-Rice <[email-masked]>
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