Subject: Re: [bug#54474] [PATCH 1/6] gnu: Add emacs-logos.

From: Nicolas Goaziou <[email-masked]>
Date: Sun, 20 Mar 2022 14:30:28 +0100

Hello,

Jai Vetrivelan <[email-masked]> writes:

> Subject: [PATCH 1/6] gnu: Add emacs-logos.
>
> * gnu/packages/emacs-xyz.scm (emacs-logos): New variable.

Thanks. Some comments follow.

>   (source (origin
>            (method git-fetch)
>            (uri (git-reference
>                  (url "https://gitlab.com/protesilaos/logos.git/")

You can drop the ".git" suffix, as mentioned by "guix lint".

> +    (arguments
> +     `(#:phases

You should use G-expressions here, if only to avoid writing

  (assoc-ref outputs "out")

which is too low level.

So:

  (arguments
   (list
    #:phases
    #~(modifiy phases ...)))

> +       (modify-phases %standard-phases
> +         (add-after 'install 'makeinfo
> +           (lambda* (#:key outputs #:allow-other-keys)

Here, this would become

  (lambda _
   ...)

> +             (invoke "emacs"
> +                     "--batch"
> +                     "--eval=(require 'ox-texinfo)"
> +                     "--eval=(find-file \"README.org\")"
> +                     "--eval=(org-texinfo-export-to-info)")
> +             (install-file "logos.info"
> +                           (string-append (assoc-ref outputs "out")
> +                                          "/share/info")))))))

And here:

  (install-file "logos.info" (string-append #$output "/share/info"))

The same goes for other packages.

Could you send updated patches?

Regards,
-- 
Nicolas Goaziou