Subject: Re: bug#26203: [PATCH] gnu: Add swaks.

From: Marius Bakke <[email-masked]>
Date: Tue, 21 Mar 2017 14:12:30 +0100

Arun Isaac <[email-masked]> writes:

> * gnu/packages/mail.scm (swaks): New variable.

Thanks!

[...]

> +    (arguments
> +     `(#:tests? #f ; No tests
> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (replace 'build
> +                  (lambda _
> +                    (zero? (system* "pod2man" "doc/ref.pod" "swaks.1"))))

You can install 'emacs-guix' and activate 'guix-devel-mode', then
indentation should work properly.

> +         (replace 'install
> +                  (lambda _
> +                    (let* ((out (assoc-ref %outputs "out"))
> +                           (bin (string-append out "/bin"))
> +                           (man (string-append out "/share/man/man1")))
> +                      (mkdir-p bin)
> +                      (install-file "swaks" bin)
> +                      (mkdir-p man)
> +                      (install-file "swaks.1" man))))

'install-file' from (guix build utils) will create the directory if it
does not exist, so the mkdir-p is redundant. In addition, 'install-file'
(or actually 'copy-file') has an unspecified return value, so we end
such phases with an explicit '#t'.

> +         (add-after 'install 'wrap-program
> +                    (lambda _
> +                      (wrap-program (string-append (assoc-ref %outputs "out")
> +                                                   "/bin/swaks")

Please use the (lambda* (#:key outputs ...)) form here, instead of
looking up %outputs directly.

> +                        `("PERL5LIB" ":" = (,(getenv "PERL5LIB")))))))))

This phase should also end on a #t for the same reason as above.

> +    (home-page "http://jetmore.org/john/code/swaks/")
> +    (synopsis "Featureful, flexible, scriptable, transaction-oriented SMTP
> +test tool")

I would shorten synopsis to something like "Featureful SMTP test tool",
since the other details are mentioned in the description.

Otherwise this LGTM, sorry for the nit-picks! Can you send an updated
patch? :)

> +    (description "Swaks is a flexible, scriptable, transaction-oriented SMTP
> +test tool.  It handles SMTP features and extensions such as TLS,
> +authentication, and pipelining; multiple version of the SMTP protocol
> +including SMTP, ESMTP, and LMTP; and multiple transport methods including
> +unix-domain sockets, internet-domain sockets, and pipes to spawned processes.
> +Options can be specified in environment variables, configuration files, and
> +the command line allowing maximum configurability and ease of use for
> +operators and scripters.")
> +    (license gpl2+)))
> -- 
> 2.11.0