From debbugs-submit-bounces@debbugs.gnu.org Sat Feb 18 04:47:19 2017 Received: (at 25728) by debbugs.gnu.org; 18 Feb 2017 09:47:19 +0000 Received: from localhost ([127.0.0.1]:44156 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cf1bm-0004Bn-QU for submit@debbugs.gnu.org; Sat, 18 Feb 2017 04:47:19 -0500 Received: from sender-of-o51.zoho.com ([135.84.80.216]:21021) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cf1bl-0004Be-Cx for 25728@debbugs.gnu.org; Sat, 18 Feb 2017 04:47:14 -0500 Received: from localhost (xd933e48a.dyn.telefonica.de [217.51.228.138]) by mx.zohomail.com with SMTPS id 1487411230597379.57264646137844; Sat, 18 Feb 2017 01:47:10 -0800 (PST) References: <20170214185339.25538-1-contact.ng0@cryptolab.net> <20170214185339.25538-2-contact.ng0@cryptolab.net> User-agent: mu4e 0.9.18; emacs 25.1.1 From: Ricardo Wurmus To: contact.ng0@cryptolab.net Subject: Re: bug#25728: [PATCH 2/2] gnu: Add colorforth. In-reply-to: <20170214185339.25538-2-contact.ng0@cryptolab.net> X-URL: https://elephly.net X-PGP-Key: https://elephly.net/rekado.pubkey X-PGP-Fingerprint: BCA6 89B6 3655 3801 C3C6 2150 197A 5888 235F ACAC Date: Sat, 18 Feb 2017 10:47:05 +0100 Message-ID: <871suvu8yu.fsf@elephly.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 25728 Cc: ng0 , 25728@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.3 (--) contact.ng0@cryptolab.net writes: > From: ng0 > > * 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