Subject: Re: [bug#31449] [PATCH 2/3] gnu: Add jupyter-guile-kernel.

From: [email-masked] (Ludovic Courtès)
Date: Mon, 14 May 2018 15:06:23 +0200

Rouby Pierre-Antoine <[email-masked]> skribis:

> * gnu/package/guile.scm (jupyter-guile-kernel): New variable.

[...]

> +                (scm-files '("hmac.scm"
> +                             "tools.scm"
> +                             "main.scm"))

Maybe (find-files "." "\\.scm$") would work?

> +                (guile-env    (if (not (eq? (getenv "GUILE_LOAD_PATH")
> +                                            #f))
> +                                  (string-append
> +                                   (getenv "GUILE_LOAD_PATH") ":")
> +                                  ""))
> +                (guile-go-env (if (not (eq? (getenv "GUILE_LOAD_COMPILED_PATH")
> +                                            #f))
> +                                  (string-append
> +                                   (getenv "GUILE_LOAD_COMPILED_PATH") ":")
> +                                  "")))

(not (eq? foo #f)) can be simplified to (not foo).

Also (getenv "GUILE_LOAD_PATH") and (getenv "GUILE_LOAD_COMPILED_PATH")
are always true in this case, I think, so perhaps you can remove these
two variables altogether?

> +           ;; Sed kernel file
> +           (substitute* (string-append kernel-dir "/"
> +                                       kernel-file)
> +             (("/home/jerry/.local/share/jupyter/kernels/guile/main.scm")
> +              (string-append module-dir "/main.scm")))

Maybe the comment could be, IIUC:

  ;; Fix hard-coded file name in the kernel.

> +    (synopsis "Guile kernel for jupyter")
                                   ^
“Guile kernel for the Jupyter Notebook”

> +    (description
> +     "This package is Guile kernel for Jupyter Notebook.  This software is
> +write for GNU Guile 2.0.")

What about:

  “This package provides a Guile 2.x kernel for the Jupyter Notebook.
  It allows users to interact with the Guile REPL through Jupyter.”

Could you send an updated patch?

Thanks!

Ludo’.