guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Maxime Devos
Subject: [bug#47582] [PATCH 2/2] gnu: Add python-pysctp.
Date: Sat, 03 Apr 2021 21:01:01 +0200
User-agent: Evolution 3.34.2

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: signature.asc
Description: This is a digitally signed message part


reply via email to

[Prev in Thread] Current Thread [Next in Thread]