Re: [PATCH] Add procmail

[Andy Wingo]

Hi Lukas,

On Sat 27 Feb 2016 10:16, Lukas Gradl <address@hidden> writes:

> I am new to Guix and Scheme, so I would very much welcome any comments
> you might have.

Welcome :)

> +       ;; the following patch fixes an ambiguous definition of
> +       ;; getline() in formail.c. The patch is provided by Debian as
> +       ;; patch 24

We tend to prefer full sentences in comments like this one, including
ending punctuation.  Also as a GNU-ism, we are in the church of
two-spaces-after-full-stops :)

> +    (arguments
> +     `(#:phases (alist-replace
> +                 'configure
> +                 (lambda _
> +                   (substitute* "Makefile"
> +                     (("/bin/sh")
> +                      (which "sh")))
> +                   (substitute* "Makefile"
> +                     (("/usr")
> +                      (assoc-ref %outputs "out")))
> +                   (substitute* "Makefile"
> +                     (("/bin/rm")
> +                      (which "rm"))))
> +                 %standard-phases)

There are many packages that still use this syntax, but the current
Best Practice™ is to use `modify-phases'.  Search Guix for some
examples.  Also, substitute* can do all these clauses in one:

  (substitute* "Makefile"
    (("/bin/sh")
     (which "sh"))
    (("/usr")
     (assoc-ref %outputs "out"))
    (("/bin/rm")
     (which "rm")))

The * in substitute* is like the * in let*, meaning the substitutions
will be done in order.  Also, the return value of substitute* is
unspecified (see the Guile manual for more), so be sure to return a true
value from this lambda by just ending the lambda with #t.

> +       #:tests? #f)) ; no tests

Please mention why the tests are disabled.

> +    (build-system gnu-build-system)
> +    (inputs `(("glibc" ,glibc)
> +              ("exim" ,exim)))
> +    (home-page "http://www.procmail.org/";)
> +    (synopsis "Versatile mail delivery agent (MDA)")
> +    (description "Procmail is a mail delivery agent (MDA) featuring support
> +for a variety of mailbox formats such as mbox, mh and maildir.  Incoming mail
> +can be sorted into separate files/directories and arbitrary commands can be
> +executed on mail arrival.  Procmail is considered stable, but is no longer
> +maintained.")
> +    (license gpl2+))) ; procmail allows to choose the
> +                      ; nonfree Artistic License 1.0
> +                      ; as alternative to the GPL2+.
> +                      ; This option is not listed here.

This sort of a comment is best done as a two-semicolon comment, I think.

Looking pretty good!

Andy