Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Thank you.

Hi Nicolas.  Thank you for your feedback!

>> +    (arguments
>
> You should use G-expressions from here:
>
> (list
>  #:modules '(...)
>  #:imported-modules `(...)
>  #:configure-flags
>  #~(list (string-append ... (emacs:elpa-directory #$output))))
>  #:phases
>  #~(modify-phases ...)

Fixed.

>> +         (add-after 'enter-lisp-dir 'emacs-patch-variables
>> +           (lambda* (#:key outputs #:allow-other-keys)
>
> This is not necessary: lambda _

I don't understand what you mean.  There is no lambda _ above -- did you
mean this snippet:

          (replace 'bootstrap
            (lambda _ (invoke "autoreconf" "-vif")))
          (add-after 'compress-documentation 'enter-lisp-dir
            (lambda _ (chdir "lisp/")))

If so, how to improve that?

>> +             (make-file-writable "eweouz.el")
>
> I think you can remove this line.

Fixed.  I got permission errors earlier, but it seems to have gone away.

>
>> +             (substitute* "eweouz.el"
>> +               (("\\(setq eweouz-helper-dirs '\\(")
>> +                (format #f "(setq eweouz-helper-dirs '(~s "
>> +                        (string-append (assoc-ref outputs "out")
>> +                                       "/libexec/eweouz"))))))
>
> You should use emacs:emacs-substitute-variables here.

I used emacs-substitute-sexp now.  It seems emacs-substitute-variables
only works on def*, and here it was setq.

>> +     "eweouz is an tool for looking up contacts from Evolution Data
>> Server
>
> Typo and capitalization: Eweouz is a tool...
>
>> +from Emacs. It is similar to BBDB, except much, much simpler.")
>
> You should separate sentences with two spaces.

Fixed.

>> +    ;; Most things are GPLv2-only although lisp/vcard.el is GPLv2+.
>> +    (license (list license:gpl2 license:gpl2+))))
>
> Could you send an updated patch?

Please see below!

/Simon