Subject: Re: bug#60369: [PATCH] gnu: Add eweouz.

From: Ludovic Courtès <[email-masked]>
Date: Tue, 31 Jan 2023 23:42:22 +0100

Hi Simon,

Simon Josefsson <[email-masked]> skribis:

> While the package builds and works for me, I would appreciate a review
> so I can learn -- I wrote this without understanding anything of what I
> was doing, but merely pattern-matched things against other existing
> packages that looked relevant.

Let’s see.

> From 52b0ad49ef0436346f53bebee4690961fba9cf90 Mon Sep 17 00:00:00 2001
> From: Simon Josefsson <[email-masked]>
> Date: Tue, 27 Dec 2022 23:38:33 +0100
> Subject: [PATCH] gnu: Add eweouz.
>
> * gnu/packages/emacs-xyz.scm (eweouz): New variable.

[...]

> +(define-public eweouz
> +  (package
> +    (name "eweouz")

Rather “emacs-eweouz” (info "(guix) Package Naming").

> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f

Please add a short comment saying why tests are disabled.

> +       (modify-phases %standard-phases
> +         (replace 'bootstrap
> +           (lambda _ (invoke "autoreconf" "-vif") #t))

Is this needed?  The default ‘bootstrap’ phase does that, roughly.  You
can omit the trailing #t too.

> +    (synopsis "Emacs interface to Evolution Data Server")
> +    (description
> +     "eweouz is an tool for looking up contacts from Evolution Data Server
> +from Emacs. It is similar to BBDB, except much, much simpler.")
> +    (license license:gpl2)))

Might be ‘gpl2+’, unless it explicitly states “version 2 only”.

That’s all I have to say!  Overall it’s looking good.  :-)

Could you send an updated patch?

Thanks,
Ludo’.