[bug#58623] [PATCH 0/3] import/cran: Parameterize for guix-cran.

[Ludovic Courtès]

Hi!

Lars-Dominik Braun <lars@6xq.net> skribis:

> the attached patches are required for guix-cran
> (https://github.com/guix-science/guix-cran). import/cran already has the
> ability to add a prefix to licenses, but it was not exposed. Additionally
> I need to parameterize fetch/download functions, so I can cache the
> tarballs/DESCRIPTION files.

Nice!  Minor comments:

> +(define %license-prefix
> +  (make-parameter identity))

Overall, unless it’s impractical, I’d suggest using explicit keyword
parameters instead of SRFI-39 parameters (like this one).  That makes
things clearer.  (That’s essentially lexical scope vs. dynamic scope.)

Also, when introducing a public global variable, please add a short
comment below the ‘define’ line explaining what it does.

> +++ b/guix/scripts/import/cran.scm
> @@ -53,6 +53,9 @@ (define (show-help)
>    (display (G_ "
>    -s, --style=STYLE      choose output style, either specification or 
> variable"))
>    (display (G_ "
> +  -p, --license-prefix=PREFIX
> +                         add custom prefix to licenses, useful for prefixed 
> import of (guix licenses)"))

I agree with zimoun that this should be documented in the manual.  I’d
also remove everything after the comma.

> +(define %fetch-description
> +  (make-parameter fetch-description))

Same comment as above.

> +(define %download-source
> +  (make-parameter download))

Ditto.

Thanks!

Ludo’.