diff mbox series

[bug#41919] Add curlpp C++ bindings to curl package

Message ID 96308bdb48d4329a55b2201b64839b1aae5da160.camel@rdmp.org
State Accepted
Headers show
Series [bug#41919] Add curlpp C++ bindings to curl package | expand

Checks

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

Commit Message

Dale Mellor June 21, 2020, 11:16 p.m. UTC
On Thu, 2020-06-18 at 14:43 +0200, Ludovic Courtès wrote:
> Hi Dale,
> 
> Dale Mellor <guix-devel-0brg6b@rdmp.org> skribis:
> 
> > From 36c628cd27c2c6f281ec800767ba0e514fae13b1 Mon Sep 17 00:00:00 2001
> > From: Dale Mellor <guix-devel-0brg6b@rdmp.org>
> > Date: Wed, 17 Jun 2020 12:42:54 +0100
> > Subject: [PATCH] gnu:  Add curlpp library.
> > 
> > * gnu/packages/curl.scm (curlpp): New variable.
> 
> Nice!  A couple of comments:
> ...


Hi Ludo, I took all your comments on board; the attached patch replaces
the one I sent earlier.

Thanks,
Dale

Comments

Marius Bakke June 24, 2020, 6:58 p.m. UTC | #1
Hello Dale,

Thanks for the updated patch.  I will suggest another few improvements,
then I think it should be good to go.  :-)

Dale Mellor <guix-devel-0brg6b@rdmp.org> writes:

> From b1506404e27552a171fe535144b2d11b6aa67cfa Mon Sep 17 00:00:00 2001
> From: Dale Mellor <guix-devel-0brg6b@rdmp.org>
> Date: Sun, 21 Jun 2020 23:53:07 +0100
> Subject: [PATCH] gnu: Add curlpp library.
>
> * gnu/packages/curl.scm (curlpp): New variable.

[...]

> @@ -32,10 +33,12 @@
>    #:use-module (guix download)
>    #:use-module (guix git-download)
>    #:use-module (guix utils)
> +  #:use-module (guix build-system cmake)
>    #:use-module (guix build-system gnu)
>    #:use-module (guix build-system go)
>    #:use-module (gnu packages)
>    #:use-module (gnu packages compression)
> +  #:use-module (gnu packages gawk)

This import is unused.

>    #:use-module (gnu packages golang)
>    #:use-module (gnu packages guile)
>    #:use-module (gnu packages kerberos)
> @@ -258,3 +261,33 @@ not offer a replacement for libcurl.")
>  Guile to do client-side URL transfers, like requesting documents from HTTP or
>  FTP servers.  It is based on the curl library.")
>     (license license:gpl3+)))
> +
> +(define-public  curlpp
                 ^^
Extra space _____|

> +  (package
> +   (name "curlpp")
> +   (version "0.8.1")
> +   (source
> +    (origin
> +             (method  git-fetch)

What happened with the indentation here?  Also, there is an extra space
between 'method' and 'git-fetch'.

> +     (uri
> +      (git-reference

Place git-reference on the same line as uri.

> +                   (url "https://github.com/jpbarrette/curlpp.git")
> +                   (commit "v0.8.1")))

Use (string-append "v" version) here instead of hard-coding.  Also, you
should add a (file-name (git-file-name name version)) in this vicinity.

> +             (sha256
> +              (base32 "1b0ylnnrhdax4kwjq64r1fk0i24n5ss6zfzf4hxwgslny01xiwrk"))))
> +   (build-system  cmake-build-system)
                   ^^
Another extra space.

> +   ;; There are no build tests to be had.
> +   (arguments
> +    '(#:tests? #f))
> +   ;; The installed version needs the header files from the C library.
> +   (propagated-inputs
> +    `(("curl" ,curl)))
> +   (synopsis  "C++ wrapper around libcURL")
               ^^
Here too ______|

> +   (description
> +    "A free and easy-to-use client-side C++ URL transfer library,

Please use full sentences in the description.  In particular, start with
"This package provides a ..." instead of jumping to the "A ...".

> +supporting FTP, FTPS, HTTP, HTTPS, GOPHER, TELNET, DICT, FILE and LDAP; in
> +particular it supports HTTPS certificates, HTTP POST, HTTP PUT, FTP uploading,
> +kerberos, HTTP form based upload, proxies, cookies, user+password
> +authentication, file transfer resume, http proxy tunneling and more!")
> +   (home-page  "http://www.curlpp.org")
> +   (license  license:x11-style)))

There are extra spaces after 'home-page' and 'license'.  Also,
license:x11-style is a procedure that takes two arguments: an URI and
optionally a comment.  However reading the source files, it looks like
the standard "Expat" license, so you can use license:expat instead.

Can you send an updated patch?  TIA!
diff mbox series

Patch

From b1506404e27552a171fe535144b2d11b6aa67cfa Mon Sep 17 00:00:00 2001
From: Dale Mellor <guix-devel-0brg6b@rdmp.org>
Date: Sun, 21 Jun 2020 23:53:07 +0100
Subject: [PATCH] gnu: Add curlpp library.

* gnu/packages/curl.scm (curlpp): New variable.
---
 gnu/packages/curl.scm | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/gnu/packages/curl.scm b/gnu/packages/curl.scm
index 48d7dd40bd..c8db551b00 100644
--- a/gnu/packages/curl.scm
+++ b/gnu/packages/curl.scm
@@ -10,6 +10,7 @@ 
 ;;; Copyright © 2018 Roel Janssen <roel@gnu.org>
 ;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net>
+;;; Copyright © 2020 Dale Mellor <guix-devel-0brg6b@rdmp.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -32,10 +33,12 @@ 
   #:use-module (guix download)
   #:use-module (guix git-download)
   #:use-module (guix utils)
+  #:use-module (guix build-system cmake)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system go)
   #:use-module (gnu packages)
   #:use-module (gnu packages compression)
+  #:use-module (gnu packages gawk)
   #:use-module (gnu packages golang)
   #:use-module (gnu packages guile)
   #:use-module (gnu packages kerberos)
@@ -258,3 +261,33 @@  not offer a replacement for libcurl.")
 Guile to do client-side URL transfers, like requesting documents from HTTP or
 FTP servers.  It is based on the curl library.")
    (license license:gpl3+)))
+
+(define-public  curlpp
+  (package
+   (name "curlpp")
+   (version "0.8.1")
+   (source
+    (origin
+             (method  git-fetch)
+     (uri
+      (git-reference
+                   (url "https://github.com/jpbarrette/curlpp.git")
+                   (commit "v0.8.1")))
+             (sha256
+              (base32 "1b0ylnnrhdax4kwjq64r1fk0i24n5ss6zfzf4hxwgslny01xiwrk"))))
+   (build-system  cmake-build-system)
+   ;; There are no build tests to be had.
+   (arguments
+    '(#:tests? #f))
+   ;; The installed version needs the header files from the C library.
+   (propagated-inputs
+    `(("curl" ,curl)))
+   (synopsis  "C++ wrapper around libcURL")
+   (description
+    "A free and easy-to-use client-side C++ URL transfer library,
+supporting FTP, FTPS, HTTP, HTTPS, GOPHER, TELNET, DICT, FILE and LDAP; in
+particular it supports HTTPS certificates, HTTP POST, HTTP PUT, FTP uploading,
+kerberos, HTTP form based upload, proxies, cookies, user+password
+authentication, file transfer resume, http proxy tunneling and more!")
+   (home-page  "http://www.curlpp.org")
+   (license  license:x11-style)))
-- 
2.26.2