GNU bug report logs - #36082
[PATCH] add gnurobots

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 36082 in the body.
You can then email your comments to 36082 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jesse Gibbons <jgibbons2357 <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] add gnurobots
Date: Mon, 3 Jun 2019 16:14:31 -0600

From 77168e843404ba8cddbace220a04d2d30d419a2f Mon Sep 17 00:00:00 2001
From: Jesse Gibbons <jgibbons2357+guix <at> gmail.com>
Date: Mon, 3 Jun 2019 16:10:43 -0600
Subject: [PATCH] add gnurobots

---
 gnu/packages/games.scm | 49 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/gnu/packages/games.scm b/gnu/packages/games.scm
index 4fdc9b01e6..6416b6b1bc 100644
--- a/gnu/packages/games.scm
+++ b/gnu/packages/games.scm
@@ -40,6 +40,7 @@
 ;;; Copyright © 2019 Oleg Pykhalov <go.wigust <at> gmail.com>
 ;;; Copyright © 2019 Pierre Langlois <pierre.langlois <at> gmx.com>
 ;;; Copyright © 2019 Julien Lepiller <julien <at> lepiller.eu>
+;;; Copyright © 2019 Jesse Gibbons <jgibbons2357+guix <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -7336,3 +7337,51 @@ Unfortunately, Hacker is not aware of Drascula's
real ambitions: DOMINATING the World and demonstrating that he is even
more evil than his brother Vlad.") ;; Drascula uses a BSD-like license.
     (license (license:non-copyleft "file:///readme.txt"))))
+
+
+(define-public gnurobots
+  (package
+   (name "gnurobots")
+   (version "1.2.0")
+   (source
+    (origin
+     (method url-fetch)
+     (uri (string-append
+	   "mirror://gnu/gnurobots/gnurobots-"
+	   version
+	   ".tar.gz"))
+     (sha256
+      (base32
+       "07gi3lsmbzzsjambgixj6xy79lh22km84z7bnzgwzxdy806lyvwb"))))
+   (build-system gnu-build-system)
+   (inputs
+    `(("glib" ,glib)
+      ("gtk+" ,gtk+-2)
+      ("vte" ,vte/gtk+-2)
+      ("readline" ,readline)
+      ("guile" ,guile-1.8)
+      ("pkg-config" ,pkg-config)))
+   (arguments '(
+		;-Werror=deprecated-declarations kills the build. Fix
it!
+		#:make-flags
'("CFLAGS=-Wno-error=deprecated-declarations")
+		#:phases
+		;readline headers were moved to readline/readline.h.
Fix before we compile.
+		(modify-phases %standard-phases
+				(add-before 'build
'patch-shell-references
+				(lambda _
+							  (begin
+
(substitute* (find-files "src" "\\.c$")
+
(("<readline.h>") "<readline/readline.h>"))
+
(substitute* (find-files "src" "\\.c$")
+
(("<history.h>") "<readline/history.h>"))))))))
+   (synopsis
+    "Program a little robot and watch him explore a world")
+   (description
+    "GNU Robots is a game in which you program a robot to explore a
world +full of enemies that can hurt it, obstacles and food to be eaten.
+The goal of the game is to stay alive and collect prizes.  The robot
+program conveniently may be written in a plain text file in the
+Scheme programming language.")
+   (home-page
+    "http://www.gnu.org/software/gnurobots/readme.html")
+   (license gpl3+)))
-- 
2.21.0

Message #8 received at 36082 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Jesse Gibbons <jgibbons2357 <at> gmail.com>
Cc: 36082 <at> debbugs.gnu.org
Subject: Re: [bug#36082] [PATCH] add gnurobots
Date: Wed, 05 Jun 2019 15:38:06 +0200

Hi Jesse,

> From 77168e843404ba8cddbace220a04d2d30d419a2f Mon Sep 17 00:00:00 2001
> From: Jesse Gibbons <jgibbons2357+guix <at> gmail.com>
> Date: Mon, 3 Jun 2019 16:10:43 -0600
> Subject: [PATCH] add gnurobots

Thank you for your patch.  It looks like the patch doesn’t apply to the
git repository.  Maybe it got mangled by your email client?

I’ll continue with some comments about the patch.

> +
> +

Please remove the extra line break.

> +(define-public gnurobots
> +  (package
> +   (name "gnurobots")
> +   (version "1.2.0")
> +   (source
> +    (origin
> +     (method url-fetch)
> +     (uri (string-append
> +	   "mirror://gnu/gnurobots/gnurobots-"
> +	   version
> +	   ".tar.gz"))
> +     (sha256
> +      (base32
> +       "07gi3lsmbzzsjambgixj6xy79lh22km84z7bnzgwzxdy806lyvwb"))))

The indentation looks off.  I can fix this for you, of course, or you
could use the automatic indenter script; see the Contributing section in
the manual for more information.

> +   (build-system gnu-build-system)
> +   (inputs
> +    `(("glib" ,glib)
> +      ("gtk+" ,gtk+-2)
> +      ("vte" ,vte/gtk+-2)
> +      ("readline" ,readline)
> +      ("guile" ,guile-1.8)
> +      ("pkg-config" ,pkg-config)))
> +   (arguments '(

Please don’t leave parentheses dangling on their own like that.  They
need company.

> +		;-Werror=deprecated-declarations kills the build. Fix
> it!

Please use “;;” for comments on their own line.  Single semicolon is
used for “margin comments”, i.e. a comment at the end of a line.

> +		#:make-flags
> '("CFLAGS=-Wno-error=deprecated-declarations")
> +		#:phases
> +		;readline headers were moved to readline/readline.h.
> Fix before we compile.

Same here.  Please also use full sentences when necessary.  I’d say no
comment is necessary here.

> +		(modify-phases %standard-phases
> +				(add-before 'build
> 'patch-shell-references

Please change the name of this build phase.  “patch-shell-references”
does not describe what this phase does.

> +				(lambda _
> +							  (begin

You don’t need “begin” here.

> +
> (substitute* (find-files "src" "\\.c$")
> +
> (("<readline.h>") "<readline/readline.h>"))
> +
> (substitute* (find-files "src" "\\.c$")
> +
> (("<history.h>") "<readline/history.h>"))))))))

You could merge these two substitutions; after all they operate on the
same set of files.  However, I think that a better approach might be to
add the “readline” sub-directory to C_INCLUDE_PATH.  So instead of
patching files you’d just set one environment variable.

> +   (synopsis
> +    "Program a little robot and watch him explore a world")

“him” –> “it”

> +   (description
> +    "GNU Robots is a game in which you program a robot to explore a
> world +full of enemies that can hurt it, obstacles and food to be eaten.
> +The goal of the game is to stay alive and collect prizes.  The robot
> +program conveniently may be written in a plain text file in the
> +Scheme programming language.")
> +   (home-page
> +    "http://www.gnu.org/software/gnurobots/readme.html")

Plesae use HTTPS and remove the “/readme.html” part.

> +   (license gpl3+)))

This should be “(license license:gpl3+)” because all values from the
(guix licenses) module are imported with a “license:” prefix.

Would you like to send a new patch?  If you aren’t comfortable with “git
send-email” please attach the generated patch file to your email (don’t
copy/paste the contents).

Thanks!

--
Ricardo

Message #11 received at 36082 <at> debbugs.gnu.org (full text, mbox):

From: Jesse Gibbons <jgibbons2357 <at> gmail.com>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 36082 <at> debbugs.gnu.org
Subject: Re: [bug#36082] [PATCH] add gnurobots
Date: Wed, 5 Jun 2019 14:10:17 -0600

[Message part 1 (text/plain, inline)]

Thanks for the review.

On Wed, 05 Jun 2019 15:38:06 +0200
Ricardo Wurmus <rekado <at> elephly.net> wrote:

> Hi Jesse,
> 
> > From 77168e843404ba8cddbace220a04d2d30d419a2f Mon Sep 17 00:00:00
> > 2001 From: Jesse Gibbons <jgibbons2357+guix <at> gmail.com>
> > Date: Mon, 3 Jun 2019 16:10:43 -0600
> > Subject: [PATCH] add gnurobots  
> 
> Thank you for your patch.  It looks like the patch doesn’t apply to
> the git repository.  Maybe it got mangled by your email client?
> 
> I’ll continue with some comments about the patch.
> 
> > +
> > +  
> 
> Please remove the extra line break.
> 
> > +(define-public gnurobots
> > +  (package
> > +   (name "gnurobots")
> > +   (version "1.2.0")
> > +   (source
> > +    (origin
> > +     (method url-fetch)
> > +     (uri (string-append
> > +	   "mirror://gnu/gnurobots/gnurobots-"
> > +	   version
> > +	   ".tar.gz"))
> > +     (sha256
> > +      (base32
> > +       "07gi3lsmbzzsjambgixj6xy79lh22km84z7bnzgwzxdy806lyvwb"))))  
> 
> The indentation looks off.  I can fix this for you, of course, or you
> could use the automatic indenter script; see the Contributing section
> in the manual for more information.
I tried the indenter script. Made the code look worse. "guix lint"
complained about the line length. Furthermore, "guix lint" complains
about the line length when I autoindent. New patch leaves scripted
indent alone.
> 
> > +   (build-system gnu-build-system)
> > +   (inputs
> > +    `(("glib" ,glib)
> > +      ("gtk+" ,gtk+-2)
> > +      ("vte" ,vte/gtk+-2)
> > +      ("readline" ,readline)
> > +      ("guile" ,guile-1.8)
> > +      ("pkg-config" ,pkg-config)))
"guix lint" says pkg-config should be a native-input, but when it is
defined as a native-input it says it fails to make derivatives for
other systems. Since I am not currently able to reconfigure my system,
I'll let someone else handle this problem.
> > +   (arguments '(  
> 
> Please don’t leave parentheses dangling on their own like that.  They
> need company.
> 
> > +		;-Werror=deprecated-declarations kills the build.
> > Fix it!  
> 
> Please use “;;” for comments on their own line.  Single semicolon is
> used for “margin comments”, i.e. a comment at the end of a line.
> 
> > +		#:make-flags
> > '("CFLAGS=-Wno-error=deprecated-declarations")
> > +		#:phases
> > +		;readline headers were moved to
> > readline/readline.h. Fix before we compile.  
> 
> Same here.  Please also use full sentences when necessary.  I’d say no
> comment is necessary here.
Not certain what you mean. The comment contained two complete sentences.
Comment removed.
> 
> > +		(modify-phases %standard-phases
> > +				(add-before 'build
> > 'patch-shell-references  
> 
> Please change the name of this build phase.  “patch-shell-references”
> does not describe what this phase does.
> 
> > +				(lambda _
> > +							  (begin  
> 
> You don’t need “begin” here.
> 
> > +
> > (substitute* (find-files "src" "\\.c$")
> > +
> > (("<readline.h>") "<readline/readline.h>"))
> > +
> > (substitute* (find-files "src" "\\.c$")
> > +
> > (("<history.h>") "<readline/history.h>"))))))))  
> 
> You could merge these two substitutions; after all they operate on the
> same set of files.  However, I think that a better approach might be
> to add the “readline” sub-directory to C_INCLUDE_PATH.  So instead of
> patching files you’d just set one environment variable.
I had trouble tredging the documentation to find out how to do this.
Some help here would be appreciated. Until we can fix it as you
suggest, the original code will be sufficient. Added TODO comment
describing the change you suggested.
> 
> > +   (synopsis
> > +    "Program a little robot and watch him explore a world")  
> 
> “him” –> “it”
Given that these descriptions are from the gnurobots authors, I don't
understand why this small change is necessary, but I'll fix.
> 
> > +   (description
> > +    "GNU Robots is a game in which you program a robot to explore a
> > world +full of enemies that can hurt it, obstacles and food to be
> > eaten. +The goal of the game is to stay alive and collect prizes.
> > The robot +program conveniently may be written in a plain text file
> > in the +Scheme programming language.")
> > +   (home-page
> > +    "http://www.gnu.org/software/gnurobots/readme.html")  
> 
> Plesae use HTTPS and remove the “/readme.html” part.
> 
> > +   (license gpl3+)))  
> 
> This should be “(license license:gpl3+)” because all values from the
> (guix licenses) module are imported with a “license:” prefix.
> 
> Would you like to send a new patch?  If you aren’t comfortable with
> “git send-email” please attach the generated patch file to your email
> (don’t copy/paste the contents).
> 
> Thanks!
> 
> --
> Ricardo
> 
Recommended fixes made where possible, though I need a bit of help with
other fixes noted above. New patch generated and attached.
--Jesse

[0001-add-gnurobots.patch (text/x-patch, attachment)]

Message #16 received at 36082-done <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Jesse Gibbons <jgibbons2357 <at> gmail.com>
Cc: 36082-done <at> debbugs.gnu.org
Subject: Re: [bug#36082] [PATCH] add gnurobots
Date: Thu, 06 Jun 2019 12:20:49 +0200

Hi Jesse,

thank you for the updated patch.  I pushed it to the master branch with
commit 08c2fb8fe9 after a couple of changes:

* fixed indentation and formatting; replaced tabs with spaces
* added a build phase to install the HOWTO document
* replaced the build phase to patch references to headers with a make
  flag
* moved pkg-config to native-inputs
* reflowed the description
* changed the commit message

Thanks!

-- 
Ricardo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.