GNU bug report logs - #34572
Add Drawpile

Previous Next

Full log

View this message in rfc822 format

From: <pkill9 <at> runbox.com>
To: "Tobias Geerinckx-Rice" <me <at> tobias.gr>
Cc: 34572 <34572 <at> debbugs.gnu.org>
Subject: [bug#34572] Add Drawpile
Date: Fri, 22 Feb 2019 07:28:24 +0000 (GMT)

[Message part 1 (text/plain, inline)]

Thanks Tobias, I removed those inline comments and removed the full stops after the other inline comments.

Haven't changed the qtcolorwidgets library config flag for now, I think drawpile upstream needs to improve the code that looks for qtcolorwidgets (https://github.com/drawpile/Drawpile/blob/master/config/FindQtColorWidgets.cmake) but I don't know how. Hardcoding the path isn't ideal but I'll leave it for now.

On Thu, 21 Feb 2019 14:58:33 +0100, Tobias Geerinckx-Rice <me <at> tobias.gr> wrote:

> 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

[0001-gnu-Add-drawpile.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.