unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Nils Gillmann <niasterisk@grrlz.net>
To: Leo Famulari <leo@famulari.name>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] gnu: add lispf4
Date: Sun, 07 Feb 2016 01:01:21 +0100	[thread overview]
Message-ID: <87io21bd7y.fsf@grrlz.net> (raw)
In-Reply-To: <20160206221446.GB6912@jasmine> (Leo Famulari's message of "Sat, 6 Feb 2016 17:14:46 -0500")

[-- Attachment #1: Type: text/plain, Size: 5205 bytes --]

Thanks for your review, the patch number 2 based on 1 should fix all the
problems you noted.

Leo Famulari <leo@famulari.name> writes:

> 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*".
>
true. fixed.
>> +         (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".
removed most comments, used install-file and copy-recursively.
>
>> +                    (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?
okay, I did not notice the intendation was off. I need to find some
visual help for that.
>
>> +                      (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.
noted, and removed most unnecessary details.
>
>> +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 guix-devel@gnu.org --cover-letter --annotate -n --thread=shallow master

I am still new to more regular patch creating with git,
so the 2nd patch has no annotations of what changed,
but it's technically still the first commit. This will
change in the near future, but currently I can only do it this way.


[-- Attachment #2: 0001-gnu-Add-lispf4.patch --]
[-- Type: text/x-patch, Size: 5132 bytes --]

From 611a064435be994874cdbaa19512f61682f55003 Mon Sep 17 00:00:00 2001
From: Nils Gillmann <niasterisk@grrlz.net>
Date: Sat, 6 Feb 2016 19:31:20 +0100
Subject: [PATCH 1/2]  gnu: Add lispf4.

    * gnu/packages/lisp.scm (lispf4): New variable.
---
 gnu/packages/lisp.scm | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/gnu/packages/lisp.scm b/gnu/packages/lisp.scm
index 726cfcd..5374483 100644
--- a/gnu/packages/lisp.scm
+++ b/gnu/packages/lisp.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2014 John Darrington <jmd@gnu.org>
 ;;; Copyright © 2015 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2016 Nils Gillmann <niasterisk@grrlz.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -27,6 +28,7 @@
   #:use-module (gnu packages texlive)
   #:use-module (gnu packages m4)
   #:use-module (guix download)
+  #:use-module (guix git-download)
   #:use-module (guix utils)
   #:use-module (guix build-system gnu)
   #:use-module (gnu packages base)
@@ -408,3 +410,73 @@ interface.")
     ;; applies to Lisp code according to them.
     (license (list license:lgpl2.1
                    license:clarified-artistic)))) ;TRIVIAL-LDAP package
+
+(define-public lispf4
+  (let* ((commit "174d8764d2f9764e8f4794c2e3feada9f9c1f1ba"))
+         (package
+          (name "lispf4")
+          (version (string-append "0.0.0-1" "-"
+                                  (string-take commit 7)))
+          (source (origin
+                    (method git-fetch)
+                    (uri (git-reference
+                          (url "https://github.com/blakemcbride/LISPF4.git")
+                          (commit commit)))
+                    (file-name (string-append name "-" version))
+                    (sha256
+                     (base32
+                      "18k8kfn30za637y4bfbm9x3vv4psa3q8f7bi9h4h0qlb8rz8m92c"))))
+          (build-system gnu-build-system)
+          ;; 80 MB appended Documentation -> output:doc
+          ;; upstream provides no `make install`
+          (outputs '("out" "doc"))
+          (arguments
+           `(#:make-flags
+             '("-f" "Makefile.unx" "CC=gcc")
+             ;; no check phase, disable it:
+             #:tests? #f
+             #: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
+                    (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"))
+                      (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.
+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'.")
+          (home-page "https://github.com/blakemcbride/LISPF4.git")
+          (license license:expat))))
-- 
2.6.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-gnu-Add-lispf4.patch --]
[-- Type: text/x-patch, Size: 6375 bytes --]

From df1d87481177f3b46efc9cde6704763397f665c4 Mon Sep 17 00:00:00 2001
From: Nils Gillmann <niasterisk@grrlz.net>
Date: Sun, 7 Feb 2016 00:44:21 +0100
Subject: [PATCH 2/2]  gnu: Add lispf4

        * gnu/packages/lisp.scm (lispf4): New variable.
---
 gnu/packages/lisp.scm | 113 ++++++++++++++++++++------------------------------
 1 file changed, 45 insertions(+), 68 deletions(-)

diff --git a/gnu/packages/lisp.scm b/gnu/packages/lisp.scm
index 5374483..cb8d923 100644
--- a/gnu/packages/lisp.scm
+++ b/gnu/packages/lisp.scm
@@ -412,71 +412,48 @@ interface.")
                    license:clarified-artistic)))) ;TRIVIAL-LDAP package
 
 (define-public lispf4
-  (let* ((commit "174d8764d2f9764e8f4794c2e3feada9f9c1f1ba"))
-         (package
-          (name "lispf4")
-          (version (string-append "0.0.0-1" "-"
-                                  (string-take commit 7)))
-          (source (origin
-                    (method git-fetch)
-                    (uri (git-reference
-                          (url "https://github.com/blakemcbride/LISPF4.git")
-                          (commit commit)))
-                    (file-name (string-append name "-" version))
-                    (sha256
-                     (base32
-                      "18k8kfn30za637y4bfbm9x3vv4psa3q8f7bi9h4h0qlb8rz8m92c"))))
-          (build-system gnu-build-system)
-          ;; 80 MB appended Documentation -> output:doc
-          ;; upstream provides no `make install`
-          (outputs '("out" "doc"))
-          (arguments
-           `(#:make-flags
-             '("-f" "Makefile.unx" "CC=gcc")
-             ;; no check phase, disable it:
-             #:tests? #f
-             #: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
-                    (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"))
-                      (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.
-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'.")
-          (home-page "https://github.com/blakemcbride/LISPF4.git")
-          (license license:expat))))
+  (let ((commit "174d8764d2f9764e8f4794c2e3feada9f9c1f1ba"))
+    (package
+      (name "lispf4")
+      (version (string-append "0.0.0-1" "-"
+                              (string-take commit 7)))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/blakemcbride/LISPF4.git")
+                      (commit commit)))
+                (file-name (string-append name "-" version))
+                (sha256
+                 (base32
+                  "18k8kfn30za637y4bfbm9x3vv4psa3q8f7bi9h4h0qlb8rz8m92c"))))
+      (build-system gnu-build-system)
+      ;; 80 MB appended Documentation -> output:doc
+      (outputs '("out" "doc"))
+      (arguments
+       `(#:make-flags
+         '("-f" "Makefile.unx" "CC=gcc")
+         ;; no check phase
+         #:tests? #f
+         #:phases
+         (modify-phases %standard-phases
+           (delete 'configure)
+           (replace
+            'install
+            (lambda* (#:key outputs inputs #:allow-other-keys)
+              (let* ((out (assoc-ref outputs "out"))
+                     (bin (string-append out "/bin")))
+                (install-file "lispf4" bin)
+                (install-file "SYSATOMS" bin)
+                (install-file "BASIC.IMG" bin)
+                (let* ((doc (assoc-ref outputs "doc"))
+                       (doc (string-append doc "/share/doc/lispf4")))
+                  (copy-recursively "Documentation" doc)))
+                #t)))))
+      (synopsis "InterLisp interpreter")
+      (description
+       "LISPF4 is an InterLisp interpreter written in FORTRAN by Mats Nordstrom
+in the early 80's.  It was converted to C by Blake McBride and supports much of
+the InterLisp Standard.  The original user manual, implementors manual and the
+documentation can be obtained through 'guix -i lispf4:doc'.")
+      (home-page "https://github.com/blakemcbride/LISPF4.git")
+      (license license:expat))))
-- 
2.6.3


[-- Attachment #4: Type: text/plain, Size: 42 bytes --]



-- 
ng/ni*
vcard: http://krosos.sdf.org

  reply	other threads:[~2016-02-07  0:01 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-06 18:45 [PATCH] gnu: add lispf4 Nils Gillmann
2016-02-06 22:14 ` Leo Famulari
2016-02-07  0:01   ` Nils Gillmann [this message]
2016-02-07 10:59     ` Ricardo Wurmus
2016-02-07 11:09     ` Ricardo Wurmus
2016-02-07 15:09       ` Nils Gillmann
2016-02-18 11:38         ` [PATCH] gnu: add lispf4 (Update) Nils Gillmann
2016-02-18 11:48           ` Ricardo Wurmus
2016-02-07 16:50 ` [PATCH] gnu: add lispf4 Nils Gillmann
2016-02-08  0:36   ` Leo Famulari
2016-02-08 11:17     ` Nils Gillmann
2016-02-08 20:14       ` Leo Famulari
2016-02-09  1:31         ` Nils Gillmann
2016-02-09  7:16           ` Efraim Flashner
2016-02-09 10:30             ` Nils Gillmann
2016-02-10  5:36               ` Leo Famulari
2016-02-10 13:48                 ` Nils Gillmann
2016-02-12  4:00                   ` Leo Famulari
2016-02-14  3:52             ` Nils Gillmann
2016-02-14  8:10               ` Ricardo Wurmus
2016-02-14 21:31                 ` Nils Gillmann
2016-02-14 21:36                   ` Ricardo Wurmus
2016-02-14  4:05             ` Nils Gillmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87io21bd7y.fsf@grrlz.net \
    --to=niasterisk@grrlz.net \
    --cc=guix-devel@gnu.org \
    --cc=leo@famulari.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).