Re: [PATCH] Add MAFFT.

[Andreas Enge]

On Tue, Aug 25, 2015 at 10:58:42PM +0200, Ludovic Courtès wrote:
> It seems this message was left unanswered.  Andreas?

I had a cursory look at the patch, here are some comments:

On Thu, Jul 23, 2015 at 10:43:31PM +1000, Ben Woodcroft wrote:
> +       #:make-flags (list (string-append "PREFIX=" (assoc-ref %outputs 
> "out"))
> +                          (string-append "BINDIR=" (string-append
> +                                                    (assoc-ref %outputs 
> "out")
> +                                                    "/bin")))

Would it make sense to include this into a 
  (let ((out (assoc-ref %outputs out))) ...)
?

> +         (add-after 'enter-dir 'patch-makefile
> +                    (lambda _
> +                      (substitute* "Makefile"
> +                        (("^SCRIPTS = mafft mafft-homologs.rb")
> +                         "SCRIPTS = mafft")
> +                        (((string-append "^PROGS = dvtditr dndfast7 dndblast"
> +                                         " sextet5 mafft-distance"))
> +                         "PROGS = dvtditr dndfast7 dndblast sextet5")

This "string-append" to concatenate two literal strings is difficult to read;
I would suggest to move the "(lambda" more to the left so that everything
fits into one line.

> +    (synopsis
> +     "Multiple sequence alignment program for unix-like operating systems")

Maybe drop "for unix-like operating systems"; instead, you could add
"for nucleotide and protein sequences" from the description.

> +protein sequences.  For instance, it offers L-INS-i (accurate; for alignment
> +of <???200 sequences) and FFT-NS-2 (fast; for alignment of <???30,000

Is this an artefact of the mailer?

I did not check whether the package passes "guix lint".

Could you send an updated patch?

Andreas