Subject: Re: [bug#30042] [PATCH 1/1] gnu: Add rct.

From: Tobias Geerinckx-Rice <[email-masked]>
Date: Tue, 9 Jan 2018 18:18:12 +0100

Fis,

I can't apply this patch (Thunderbird saves it as some kind of encoded
blob, probably as my punishment for still using Thunderbird) but I'll
assume you've tested it more thoroughly than I could anyway.

Fis Trivial wrote on 09/01/18 at 15:00:
> * gnu/packages/cpp.scm (rct): New public variable.
> ---
>  gnu/packages/cpp.scm | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/gnu/packages/cpp.scm b/gnu/packages/cpp.scm
> index 5ad9fd33b..cd058f62a 100644
> --- a/gnu/packages/cpp.scm
> +++ b/gnu/packages/cpp.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2017 Ethan R. Jones <[email-masked]>
> +;;; Copyright © 2018 Fis Trivial <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -20,9 +21,15 @@
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix packages)
>    #:use-module (guix download)
> +  #:use-module (guix git-download)
> +  #:use-module (guix build-system cmake)
>    #:use-module (guix build-system gnu)
>    #:use-module (gnu packages)
> -  #:use-module (gnu packages autotools))
> +  #:use-module (gnu packages autotools)
> +  #:use-module (gnu packages check)
> +  #:use-module (gnu packages compression)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages tls))
> 
>  (define-public libzen
>    (package
> @@ -57,3 +64,32 @@
>  strings, configuration, bit streams, threading, translation, and cross-platform
>  operating system functions.")
>      (license license:zlib)))
> +
> +(define-public rct
> +  (package
> +    (name "rct")
> +    (version "2017.12.09")

I can't find this version format in (a quick scan of) the commit log.

It's usually better to use ‘0.0.0’-based versioning (using git-version)
in case upstream decides to do proper releases at some point. Otherwise
the package might not be upgraded. There's an example in the manual.

> +    (home-page "https://github.com/Andersbakken/rct")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url home-page)
> +                    (commit "b3e6f41d9844ef64420e628e0c65ed98278a843a")))
> +              (sha256
> +               (base32
> +                "1m2931jacka27ghnpgf1z1plkkr64z0pga4r4zdrfpp2d7xnrdvb"))
> +              (file-name (git-file-name name version))))
> +    (build-system cmake-build-system)
> +    (arguments
> +     '(#:configure-flags
> +       '("-DWITH_TESTS=ON")))

It borders on the obvious, but I'd have added ‘; run the test suite’
anyway. That's up to you.

> +    (native-inputs                      ; All native-inputs are for tests
> +     `(("pkg-config" ,pkg-config)
> +       ("cppunit" ,cppunit)
> +       ("zlib" ,zlib)
> +       ("openssl" ,openssl)))

Nitpick: please keep these alphabetical whenever possible, like you did
(thanks!) with module imports above.

> +    (synopsis "C++ library providing Qt-like APIs on top of STL")

...‘the STL’?

> +    (description "Rct is a set of C++ tools that provide nicer (more Qt-like)
> + APIs on top of stl classes.")

s|stl|Standard Template Library (@dfn{STL})|

(No need to add ‘the’ in this case.)

> +    (license (license:non-copyleft "LICENSE.txt"
> +                                   "See LICENSE.txt in the distribution."))))

LICENSE.txt looks like BSD-4 to me (ugh, no licence headers... :-/).
Also, cJSON/ is expat-licenced:

  (license (list license:expat     ; cJSON/
                 license:bsd-4     ; everything else (LICENSE.txt)

The rest looks good to me. Thanks!

Kind regards,

T G-R