[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#25728: [PATCH 2/2] gnu: Add colorforth.
From: |
Ricardo Wurmus |
Subject: |
bug#25728: [PATCH 2/2] gnu: Add colorforth. |
Date: |
Sat, 18 Feb 2017 10:47:05 +0100 |
User-agent: |
mu4e 0.9.18; emacs 25.1.1 |
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.
Could you write a description that describes the package, i.e. tells
potential users why they would want to use it?
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