Subject: Re: [bug#36602] [PATCH] Add node-build-system.

From: Ludovic Courtès <[email-masked]>
Date: Sat, 13 Jul 2019 14:42:38 +0200

Hello!

Nice work!  I wonder if this could be used for the ‘node-semver’ package
that arrived at about the same time: <https://issues.guix.gnu.org/issue/36599>.

Jelle Licht <[email-masked]> skribis:

>> +(define* (install #:key outputs inputs global? #:allow-other-keys)
>> +  "Install the node module to the output store item. MODULENAME defines
>> +under which name the module will be installed, GLOBAL? determines whether this
>> +is an npm global install."
>> +  (let* ((out          (assoc-ref outputs "out"))
>> +         (src-dir      (getcwd))
>> +         (tgt-dir      (string-append out "/lib"))
>> +         (bin-dir      (string-append out "/bin"))
>> +         (modulename   (string-append  (assoc-ref (read-package-data) "name")))
>> +         (data         (read-package-data))
>> +         (bin-conf     (assoc-ref data "bin"))
>> +         (dependencies (match (assoc-ref data "dependencies")
>> +                         ((@ deps ...) deps)

Note that ‘@’ here matches anything.  Did you mean '@, which would match
the @ symbol?

>> +                         (#f #f))))
> It might be better to write out most of these
> names. I think we could also move `modulename' one line lower, so it can
> become `(modulename (assoc-ref data "name"))'.

I’m gratuitously nitpicking as well :-), but I think we should spell out
names in general, so I’d propose s/src-dir/source/, s/tgt-dir/target/,
etc.

  https://www.gnu.org/software/guix/manual/en/html_node/Formatting-Code.html

Anyway, thanks for working on it!

Ludo’.