Subject: Re: [bug#62375] [PATCH v3] import: Add binary npm importer.

From: Ludovic Courtès <[email-masked]>
Date: Mon, 01 Apr 2024 22:41:26 +0200

Hi!

[email-masked] skribis:

> From: Jelle Licht <[email-masked]>
>
> * guix/scripts/import.scm: (importers): Add "npm-binary".
> * guix/import/npm-binary.scm: New file.
> * guix/scripts/import/npm-binary.scm: New file.
> * Makefile.am: Add them.
>
> Co-authored-by: Timothy Sample <[email-masked]>
> Co-authored-by: Lars-Dominik Braun <[email-masked]>
>
> Change-Id: I98a45068cf5b9c42790664cc743feaa7ac76f807

Yay!

> +The npm-binary importer also allows you to specify a version string:
> +
> +@example
> +guix import npm-binary buffer-crc32 1.0.0
> +@end example

For consistency with other importers (pypi, gem, cran), could you change
the syntax to:

  guix import npm-binary [email-masked]

?

That’s the last remaining issue for me.

> +;; TODO: Support other registries
> +(define %registry "https://registry.npmjs.org")

For the purposes of tests, you could make it:

  (define %npm-registry
    (make-parameter "https://registry.npmjs.org"))

That would allow you to write tests using ‘with-http-server’ and
‘parameterize’ as done in ‘tests/pypi.scm’ and others, which I find
nicer and more robust than ‘mock’.

Not a blocker though.

Thanks!

Ludo’.