Subject: Re: bug#50217: [PATCH] gnu: Add c2ffi-11.0.0.

From: Ludovic Courtès <[email-masked]>
Date: Tue, 07 Sep 2021 16:06:18 +0200

Hello,

muradm <[email-masked]> skribis:

> * gnu/packages/cpp.scm (c2ffi): New variable.

Overall LGTM, modulo the following minor issues:

[...]

> +(define-public c2ffi
> +  (let ((commit "f50243926a0afb589de1078a073ac08910599582")
> +        (revision "1")
> +        (c2ffi-sha256 "17j73yb6v3lfvwwicl2f3yg141iz2dk1rf874qzqs5bkq64ldjsj"))

Normally, we only provide upstream releases.  If you think we need this
specific commit, could you add a comment explaining why?

Please leave the base32 sha256 string where it’s used, within the
(base32 …) expression (it’s a macro that decodes base32 at
compile-time.)

> +         #:tests? #f))

It would be best to run tests.  Alternatively, if this is not feasible,
please add a comment explaining why we’re skipping them.

> +      (synopsis "Clang-based FFI wrapper generator")
> +      (description "tool for extracting definitions from C, C++, and
> +Objective C headers for use with foreign function call interfaces.")

Please write full sentences, as per
<https://guix.gnu.org/manual/en/html_node/Synopses-and-Descriptions.html>.

> +      (license license:gpl2))))

Is it GPLv2-only?  Unless it’s explicitly stated, it’s probably
GPLv2-or-later (‘license:gpl2+’).

Could you send an updated patch?

Thanks!

Ludo’.