Re: [PATCH] gnu: add lispf4

[Leo Famulari]

On Sat, Feb 06, 2016 at 07:45:53PM +0100, Nils Gillmann wrote:

[...]

>     * gnu/packages/lisp.scm (lispf4): New variable.

[...]

> +(define-public lispf4
> +  (let* ((commit "174d8764d2f9764e8f4794c2e3feada9f9c1f1ba"))

I believe you can use "let" instead of "let*".

> +         (package
            ^
Can you alter the indentation so that this ( is underneath the e in let?
This will help with line length issues. (I usually refer to the package
definition for fxtract when fetching from git repos.)

[...]

> +             #:phases
> +             (modify-phases %standard-phases
> +               (delete 'configure)
> +               (replace
> +                'install
> +                (lambda* (#:key outputs inputs #:allow-other-keys)
> +                  (let* ((out (assoc-ref outputs "out"))
> +                         (bin-dir (string-append out "/bin")))
> +                    ;; Make directories
> +                    (mkdir-p bin-dir)
> +                    ;; copy files

We have a function install-file (guix/build/utils.scm) that combines
mkdir-p and copy-file. It reduces the boilerplate a bit. Could you use
that? Also we probably don't need comments such as "copy file".

> +                    (copy-file "lispf4"
> +                               (string-append bin-dir "/lispf4"))
> +                    (copy-file "SYSATOMS"
> +                               (string-append bin-dir "/SYSATOMS"))
> +                    (let* ((doc (assoc-ref outputs "doc"))
> +                           (doc-dir (string-append doc "/share/doc/lispf4")))
> +                      ;; Make directory
> +                      (mkdir-p doc-dir)
> +                      (copy-file "Documentation/DevelopmentProcess.txt"
> +                                 (string-append doc-dir 
> "/DevelopmentProcess.txt"))
> +                      (copy-file "Documentation/Haraldson-LISP_details.pdf"
> +                                 (string-append doc-dir 
> "/Haraldson-LISP_details.pdf"))

For some reason the linter doesn't complain about this line (and others)
being too long. I guess it measures from the beginning of 'package'
rather than the first column. Can you make sure they are all ≤ 80
characters after changing the indentation as requested above?

> +                      (copy-file "Documentation/ImplementationGuide.txt"
> +                                 (string-append doc-dir 
> "/ImplementationGuide.txt"))
> +                      (copy-file "Documentation/Interlisp-Oct_1978.pdf"
> +                                 (string-append doc-dir 
> "/Interlisp-Oct_1978.pdf"))
> +                      (copy-file "Documentation/p501-schorr.pdf"
> +                                 (string-append doc-dir "/p501-schorr.pdf"))
> +                      (copy-file "Documentation/README.txt"
> +                                 (string-append doc-dir "/README.txt"))
> +                      (copy-file "Documentation/UsersGuide.txt"
> +                                 (string-append doc-dir "/UsersGuide.txt")))
> +                    #t))))))
> +          (synopsis "InterLisp interpreter")
> +          (description
> +           "LISPF4 is an InterLisp interpreter written in FORTRAN by Mats 
> Nordstrom
> + (Uppsala, Sweden) in the early 80's.  It was converted to C by Blake 
> McBride.
     ^
     We probably don't need to provide this level of historical detail.

> +It supports much of the InterLisp Standard.  Interlisp is a dynamically
> +scoped lisp system.  It supports LAMBDA (evaluates function arguments),
> +NLAMBDA (doesn't evaluate its arguments), and variable number of arguments.
> +  Macros are supported as well.  The original user manual, implementors 
> manual
> +and the documentation can be obtained through 'guix -i lispf4:doc'.")

I don't think it's necessary to provide usage tips in the description.

> +          (home-page "https://github.com/blakemcbride/LISPF4.git";)
> +          (license license:expat))))
> -- 
> 2.6.3
> 
> (is there a prefered way btw? .patch file attached or just file 
> inserted/pasted?)

When using `git send-email`, the patches are automatically sent in their
own messages, which makes it trivial to apply them with `git am`.  Any
other method introduces some friction.

I typically do all my work on branches, so this works for me:
$ git send-email --to address@hidden --cover-letter --annotate -n 
--thread=shallow master