Hello, thank you for reviewing my patch.

Am 14.04.2017 um 10:07 schrieb Arun Isaac:
> 
> 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?
> 
My thought is that this way the build won't break in the same moment a
new version is released, since the referenced tar 404s when they move it.

>>> +    (arguments
>>> +     '(#:tests? #f
> 
> The release tarball does seem to come with tests. Could you package them
> as well?
> 
Ah, I missed those because they are in make test and not make check.
I tried to package them now, but the tests need to create a directory in
home. This fails and I don't know how to handle this with guix.
I need some help here.

>>> +       #:phases
>>> +       (modify-phases
>>> +           %standard-phases
> 
> Please put modify-phases and %standard-phases on the same line like:
> 
> (modify-phases %standard-phases
> 
Okay, done.

>>> +         (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.
> 
Yes, it works great!
I didn't know about that before, thank you.

>>> +         (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
> 
100% right!
I also removed the second substitute, since fonts are only required for
the graphical version.

I started packaging this more than half a year ago and only recently
decided to give it another try and remove the graphical version. Seems
like I still missed out on removing a lot of the mess.

>>> +    (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?
> 
> 
> 
I did both now. I hope this is the right way.

I'm appending my current WIP version.
The test phase is broken and I'll need help, as I said above.
When I disable the tests, the game builds and can be played.