bug#25728: [PATCH 2/2] gnu: Add colorforth.

[ng0]

Ricardo Wurmus transcribed 3.5K bytes:
> 
> address@hidden writes:
> 
> > From: ng0 <address@hidden>
> >
> > * gnu/packages/forth.scm (colorforth): New variable.
> > ---
> 
> The patch to change the module name is fine (although I’d move the
> copyright update to this patch).
> 
> >  gnu/packages/forth.scm | 38 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/gnu/packages/forth.scm b/gnu/packages/forth.scm
> > index 21a0fc2de..8854a9246 100644
> > --- a/gnu/packages/forth.scm
> > +++ b/gnu/packages/forth.scm
> > @@ -21,7 +21,9 @@
> >    #:use-module ((guix licenses) #:prefix license:)
> >    #:use-module (guix packages)
> >    #:use-module (guix download)
> > +  #:use-module (guix git-download)
> >    #:use-module (guix build-system gnu)
> > +  #:use-module (gnu packages assembly)
> >    #:use-module (gnu packages m4))
> >  
> >  (define-public gforth
> > @@ -58,3 +60,39 @@ and history.  A generic virtual machine environment, 
> > vmgen, is also
> >  included.")
> >      (home-page "https://www.gnu.org/software/gforth/";)
> >      (license license:gpl3+)))
> > +
> > +(define-public colorforth
> > +  (let ((commit "94aec438f1ded202681f18801b98c52dc3beee41")
> > +        (revision "1"))
> > +    (package
> > +      (name "colorforth")
> > +      (version (string-append "0.0.0-" revision "." (string-take commit 
> > 7)))
> > +      (source (origin
> > +                (method git-fetch)
> > +                (uri (git-reference
> > +                      (url "https://github.com/narke/colorForth";)
> > +                      (commit commit)))
> > +                (sha256
> > +                 (base32
> > +                  
> > "0s602k568bm6vmvpahsms77liicg38vksn59j5m8ax4h9l9ca77r"))))
> > +      (arguments
> > +       `(#:tests? #f
> > +         #:phases
> > +         (modify-phases %standard-phases
> > +           (delete 'configure) ; no configure script
> > +           (replace 'install ; There is no 'install
> 
> Please change the comment to “no install target” or similar.  “no
> 'install” is confusing because “'install” is a quoted symbol and that
> has no meaning outside of Scheme.
> 
> > +             (lambda _
> > +               (install-file "cf2012.img"
> > +                             (string-append (assoc-ref %outputs "out")
> > +                                            "/bin")))))))
> 
> Please use “outputs” instead of “%outputs”.  Is the target “bin”
> directory created during the build?
> 
> Please also make the phase end with “#t”.
> 
> > +      (native-inputs
> > +       `(("nasm" ,nasm)))
> > +      (build-system gnu-build-system)
> > +      (home-page "https://github.com/narke/colorForth";)
> > +      (synopsis "Native 32-bit colorForth for PCs, Bochs and Qemu")
> > +      (description
> > +       "Native colorForth for 32-bit PCs, at least compilable on Linux
> > + and runnable on both Bochs and Qemu.  It is adapted from
> > + @url{http://sourceforge.net/projects/colorforth, colorforth}.
> > + The original colorforth is public domain software.")
> 
> Please change the description.  The first sentence fragment should be a
> full sentence.  I don’t think “32-bit PCs” should be mentioned, nor
> should compatibility with Linux be mentioned (do they mean the kernel or
> GNU?).  Also the last sentence should not be included.

Okay.

> Could you write a description that describes the package, i.e. tells
> potential users why they would want to use it?

I will try, I'm not a colorforth user myself.

> Looks like it’s written in x86 assembly.  This would be worth mentioning
> (and I think that’s what “32-bit PCs” implied).
> 
> Could you please send an updated patch?
> 
> -- 
> Ricardo
> 
> GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
> https://elephly.net
> 

-- 
PGP and more: https://people.pragmatique.xyz/ng0/