[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