[bug#36082] [PATCH] add gnurobots

[Ricardo Wurmus]

Hi Jesse,

> From 77168e843404ba8cddbace220a04d2d30d419a2f Mon Sep 17 00:00:00 2001
> From: Jesse Gibbons <address@hidden>
> 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