Subject: Re: bug#26488: [PATCH] gnu: Add crawl.

From: Arun Isaac <[email-masked]>
Date: Fri, 14 Apr 2017 13:37:01 +0530

Thanks for the patch!

>> +              (uri (list
>> +                    ;; Older releases get moved into a versioned directory
>> +                    (string-append "http://crawl.develz.org/release/"
>> +                                   (version-major+minor version) "/stone_soup-"
>> +                                   version "-nodeps.tar.xz")
>> +                    ;; Only the latest release is in this directory
>> +                    (string-append "http://crawl.develz.org/release/stone_soup-"
>> +                                   version "-nodeps.tar.xz")))

Why do we need two URIs? Shouldn't the latest release alone be enough?
Isn't that the only release we need to build?

>> +    (arguments
>> +     '(#:tests? #f

The release tarball does seem to come with tests. Could you package them
as well?

>> +       #:phases
>> +       (modify-phases
>> +           %standard-phases

Please put modify-phases and %standard-phases on the same line like:

(modify-phases %standard-phases

>> +         (add-after
>> +          'unpack 'prepare-before-make
>> +          (lambda* (#:key inputs #:allow-other-keys)
>> +            (chdir "source")))

I haven't actually tried building the package, but is this phase
necessary? If you do want to change directory before running make, you
can pass "-C" and "source" as #:make-flags.

>> +         (add-before
>> +          'configure 'patch-makefile
>> +          (lambda* (#:key inputs #:allow-other-keys)
>> +            (substitute*
>> +                "Makefile"
>> +              (("SQLITE_INCLUDE_DIR := /usr/include")
>> +               "SQLITE_INCLUDE_DIR := ${sqlite}/include"))
>> +            (substitute*
>> +                "Makefile"
>> +              (("/usr/share/fonts /usr/local/share/fonts /usr/*/lib/X11/fonts;")
>> +               "${dejavu_fonts}/share/fonts;"))))
>> +         (delete 'configure))))

You should be able to do this using #:make-flags. See
https://www.gnu.org/software/make/manual/html_node/Overriding.html

>> +    (description "A roguelike adventure through dungeons filled with dangerous
>> +monsters in a quest to find the mystifyingly fabulous Orb of Zot.
>> +The game is also known under the name 'Dungeon Crawl Stone Soup'.")

Could you make this "Dungeon Crawl Stone Soup is a roguelike adventure
... "? Also, remove the last sentence "The game is also known as ...".

>> +    (license license:gpl2+)))

licence.txt mentions multiple licenses. Could you mention them all as a
list of licenses?