GNU bug report logs - #36182
[PATCH] add freeorion

Previous Next

Full log

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

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Jesse Gibbons <jgibbons2357 <at> gmail.com>
Cc: 36182 <at> debbugs.gnu.org
Subject: Re: [bug#36182] [PATCH] add freeorion
Date: Wed, 12 Jun 2019 21:48:52 +0200

Hello,

Jesse Gibbons <jgibbons2357 <at> gmail.com> writes:

> Patch is attached. -Jesse

Thank you! Some comments follow.

> From cca52f508e84ac34e60a3c5167554d7ad2ad6564 Mon Sep 17 00:00:00 2001
> From: Jesse Gibbons <jgibbons2357+guix <at> gmail.com>
> Date: Wed, 12 Jun 2019 10:07:32 -0600
> Subject: [PATCH] add freeorion

The commit message is incomplete. It should also include

    * gnu/packages/games.scm (freeorion): New variable.

If you use Emacs, there is a template to automatically fill this.

> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/freeorion/freeorion.git")
> +             (commit "470d0711537804df3c2ca25532f674ab4bec58af")))

Why do you need to use the latest commit instead of the latest stable
release? Unless there is a good reason, Guix prefers using stable
releases:

    (commit version)

> +       (sha256
> +        (base32
> +         "1wsw632l1cj17px6i88nqjzs0dngp5rsr67n6qkkjlfjfxi69j0f"))))
> +    (arguments
> +     '(#:tests? #f))

You should include a comment explaining why tests are removed. It could
be, for example,

    '(#:tests? #f))   ;no test 

> +    (home-page
> +     "https://www.freeorion.org/index.php/Main_Page")

I think "https://www.freeorion.org" is enough, since it points to the
page above.

> +    (description
> +     "FreeOrion is a free, open source, turn-based space empire and galactic
> +conquest (4X) computer game being designed and built by the FreeOrion project.
> +FreeOrion is inspired by the tradition of the Master of Orion games, but is not
> +a clone or remake of that series or any other game.")

It may be useful to explain what the "4X" means, or do not include it at
all. Note that in the wiki, "4X" is a link, so you can get further
information.

> +    (license (list license:gpl2 license:cc-by-sa3.0))))

Could you explain, in a comment, why there are two licenses? I assume
the former is the project, and the latter is for assets.

Would you mind sending an updated patch?

Regards,

-- 
Nicolas Goaziou

Previous Next

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