[bug#47582] [PATCH 2/2] gnu: Add python-pysctp.

[Maxime Devos <maximedevos@telenet.be>]

[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]

On Sat, 2021-04-03 at 19:37 +0200, Hartmut Goebel wrote:
> Am 03.04.21 um 18:12 schrieb Maxime Devos:
> > Phases do not need to return #t anymore.  IIUC the warning message that results
> > if it is left out has been removed on core-updates.
> 
> Okay, will do. Any other remarks?

About the following code:

> +           (substitute* "setup.py"
> +             (("include_dirs\\s*=.*")
> +              (string-append "include_dirs = ['.'] + '"
> +                             (getenv "C_INCLUDE_PATH") "'.split(':'),"))
> +             (("library_dirs\\s*=.*")
> +              (string-append "library_dirs = '"
> +                             (getenv "LIBRARY_PATH") "'.split(':'),")))

When cross-compiling, this code should most likely use CROSS_C_INCLUDE_PATH
and CROSS_LIBRARY_PATH instead.  (Admittedly, python-build-system does not
support cross-compilation yet, so this is not important ... yet.)

Suggestion: replace (getenv "C_INCLUDE_PATH") with
(getenv ,(if (%current-target-system)
             "CROSS_C_INCLUDE_PATH"
             "C_INCLUDE_PATH")).
Likewise for LIBRARY_PATH.

Some aesthetic nitpicks (YMMV):

> +  (synopsis  "Python module for the SCTP protocol stack and library")
A space has been doubled.

> * gnu/packages/networking.scm(python-pysctp): New variable.

I would put a space between .scm and (python-pysctp).  Most commit messages
do.

+(define-public python-pysctp
+(package
+  (name "python-pysctp")
+  ...

Indentation is wrong here.  See 16.5.4 Formatting Code for how to automatically
indent code.

Greetings,
Maxime.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]