On Sat, Apr 25, 2020 at 08:15:56PM -0600, Kozo wrote:
> Subject: [PATCH] yarnpkg
> 
> ---
>  yarn.scm | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 yarn.scm

Please write commit logs in the ChangeLog format [0]. You can check the
commit history for examples.

> (define-module (yarn)

Please put the file in gnu/packages/. Also, add it to the list in
gnu/local.mk.

>   #:use-module (guix packages)
>   #:use-module (guix download)
>   #:use-module (guix build-system trivial)
>   #:use-module (guix licenses)
>   #:use-module (gnu packages node)
>   #:use-module (ice-9 pretty-print))

I can't see where (ice-9 pretty-print) is being used.

> (define yarn-version "1.22.4")

What is the purpose of this single-use variable?

> (define-public yarn
>   (package
>    (name "yarn")
>    (version yarn-version)
>    (source (origin
>     (method url-fetch/tarbomb)

The file has only a single directory, so why use /tarbomb?

>     (uri (string-append "https://github.com/yarnpkg/yarn/releases/download/v"
>      version "/yarn-v" version ".tar.gz"))

When looking through this download, it seems that the lib/cli.js file is
a preprocessed mess. Ideally, we would build the package from source.

>     (sha256
>      (base32
>       "0n7vhwjz3lyjnavcaw08cqa8gfampqsy5mm3f555cbqb26m1clxw"))))
>    (build-system trivial-build-system)
>    (outputs '("out"))
>    (inputs `(("node" ,node)))
>    (arguments
>     `(#:modules ((guix build utils))
>       #:builder (begin
>   (use-modules (guix build utils))
>                   (let* ((out (assoc-ref %outputs "out"))
>    (bin (string-append  out "/bin"))
>    (lib (string-append  out "/lib"))

The indentation is quite confusing here, I'd suggest running
./etc/format-code.el on your file.

>    (synopsis "Dependency management tool for JavaScript")
>    (description "Fast, reliable, and secure dependency management tool
> for JavaScript.  Acts as a drop-in replacement for NodeJS's npm.")

I think there are a lot of buzzwords in this description: "fast,
reliable and secure" is not an objective property. Is there something
more neutral that could be said about the package?

Thanks,
Jakub Kądziołka

[0]: https://www.gnu.org/prep/standards/html_node/Change-Logs.html#Change-Logs