guix-patches
[Top][All Lists]
Advanced

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

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


From: Jelle Licht
Subject: [bug#62375] [PATCH v3] import: Add binary npm importer.
Date: Tue, 02 Apr 2024 16:12:01 +0200

Hi Ludo',

Thanks for the speedy review!

Ludovic Courtès <ludo@gnu.org> writes:

> Hi!
>
> jlicht@fsfe.org skribis:
>
>> From: Jelle Licht <jlicht@fsfe.org>
>>
>> * 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 <samplet@ngyro.com>
>> Co-authored-by: Lars-Dominik Braun <lars@6xq.net>
>>
>> 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 buffer-crc32@1.0.0
>
> ?
>
> That’s the last remaining issue for me.
>

I needed some custom logic to support the npm scoped packages
("@linthtml/linthtml@1.2.3"), but it should now work.

>> +;; 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";))

Easy!

> 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’.

The `with-http-server' construct does not play well with an interactive
REPL workflow due to not cleaning up the used port in all situations,
making that port unavailable for running the test again[1]. `mock',
brittle as it may be, does not to suffer from this drawback, so I'd
vastly prefer using that for now.

> Not a blocker though.
>
> Thanks!
>
> Ludo’.

I'll send a v4, let it simmer for some days and then merge it if no big
blockers or requested changes show up.

- Jelle





reply via email to

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