unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Alberto EFG <albertoefg@posteo.mx>
Cc: 40035@debbugs.gnu.org
Subject: [bug#40035] Add widelands game
Date: Thu, 12 Mar 2020 15:31:20 +0100	[thread overview]
Message-ID: <87imj9skzr.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87o8t2gpqx.fsf@posteo.mx> (Alberto EFG's message of "Wed, 11 Mar 2020 22:29:26 -0600")

Hello,

Alberto EFG <albertoefg@posteo.mx> writes:

> This is my first patch.

Thank you, and congratulations!

> +(define-public widelands
> +  (let ((commit "d9513d413f2558f9ef6f033a7685bf9881fbdbb3")
> +	(revision "1"))

I suggest to add a comment explaining why we rely on a commit, and, if
that makes sense, why this particular one, e.g., "No official release."

> +    (package
> +     (name "widelands")
> +     (version (git-version "20" revision commit))

Where is this "20" coming from?

> +     (source (origin

Nitpick: I suggest to move `origin' to a line on its own.

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

Why are the tests disabled? We usually provide a comment when disabling
tests.

> +	#:configure-flags
> +	(let* ((out (assoc-ref %outputs "out"))
> +	       (share (string-append out "/share")))
> +	  (list    "-DCMAKE_BUILD_TYPE=Release"
> +		   (string-append "-DCMAKE_INSTALL_PREFIX=" out "/bin")
> +		   (string-append "-DWL_INSTALL_BASEDIR=" share "/widelands")
> +		   (string-append "-DWL_INSTALL_DATADIR=" share "/widelands")
> +		   "-DOPTION_BUILD_WEBSITE_TOOLS=OFF"))))
> +     (inputs
> +      `(("sdl" ,(sdl-union (list sdl2
> +				 sdl2-image
> +				 sdl2-mixer
> +				 sdl2-ttf)))

Nitpick: all can go into a single line.

> +	("gettext" ,gettext-minimal)

This probably belongs to `native-inputs' not `inputs'.

> +        ("icu4c" ,icu4c)

Indentation is off here.

> +	("libpng" ,libpng)
> +	("zlib" ,zlib)
> +	("boost" ,boost)
> +	("python" ,python)

This one may also be a native input. Could you check?

> +	("glew" ,glew)))

Could you re-order inputs alphabetically?

> +     (synopsis "Real-time strategy game")

It is a bit terse. Debian uses the slightly more accurate:

  "Fantasy real-time strategy game"

Maybe it is worth mentioning. Or better, something like:

   "Fantasy real-time strategy game with singleplayer campaigns and multiplayer mode"

It could make sense since in the description I suggest below, there is
no reference to campaigns nor multiplayer.

> +     (description
> +      "Widelands is a free, open source real-time strategy game with

You can remove "free" and "open source". All is free in Guix!

> + singleplayer campaigns and a multiplayer mode.  The game was inspired
> + by Settlers II but has significantly more variety and depth to it.  ")

Mind the spurious spaces at the end.

Again, Debian uses:

    Widelands is a strategy game aiming for gameplay similar to Settlers II by
    BlueByte.

    In this game, you start out on a small piece of land with nothing more than
    a few of useful resources.  Using those, you can build yourself an empire
    with many thousands of inhabitants.  On your way towards this goal, you will
    have to build up an economic infrastructure, explore the lands around you
    and face enemies who are trying to rule the world just like you do.

Would it be better to use it?

> +     (home-page "https://www.widelands.org/")

Nitpick: home-page is usually located above synopsis. I don't know if
there's a strong rule about it, tho.

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

You are missing out some licenses (CC-based) from the assets in the
game. Could you add them too?

Could you send an updated patch?

Regards,

-- 
Nicolas Goaziou

  reply	other threads:[~2020-03-12 14:32 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-12  4:29 [bug#40035] Add widelands game Alberto EFG
2020-03-12 14:31 ` Nicolas Goaziou [this message]
2020-03-13  3:14   ` Alberto EFG
2020-03-13  8:16     ` Nicolas Goaziou
2020-06-18 21:22       ` Nicolas Goaziou
2020-06-18 22:58         ` Jonathan Brielmaier
2020-06-19  7:01           ` Nicolas Goaziou
2020-06-21  6:33           ` Efraim Flashner
2020-06-19 12:09         ` Tobias Geerinckx-Rice via Guix-patches via
2020-06-19 18:59           ` Nicolas Goaziou
2020-06-19 20:46             ` Nicolas Goaziou
2020-06-19 21:14             ` Tobias Geerinckx-Rice via Guix-patches via
2020-06-19 21:55               ` Nicolas Goaziou
2020-06-30 15:50                 ` bug#40035: " Nicolas Goaziou
2020-06-19 12:30 ` [bug#40035] " Tobias Geerinckx-Rice via Guix-patches via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87imj9skzr.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=40035@debbugs.gnu.org \
    --cc=albertoefg@posteo.mx \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).