[bug#34189] [PATCH] gnu: Add arcan.

["Ludovic Courtès" <ludo@gnu.org>]

Hello,

Sorry for the delay!  Here are some comments.

L  p R n  d n    <guix@lprndn.info> skribis:

>>From f1d5416c9d7a8a5ba5ecb4d380d06ee066b62a29 Mon Sep 17 00:00:00 2001
> From: Lprndn <guix@lprndn.info>
> Date: Thu, 24 Jan 2019 13:29:21 +0100
> Subject: [PATCH] gnu: Add arcan.
>
> * gnu/packages/arcan.scm New module.
> * gnu/packages/arcan.scm (arcan): New variable.

Should be just:

  * gnu/packages/arcan.scm: New file.

> +++ b/gnu/packages/arcan.scm
> @@ -0,0 +1,130 @@
> +(define-module (gnu packages arcan)

Please add the standard GPLv3+ header with a copyright line for yourself.

> +  (package
> +    (name "arcan")
> +    (version "0.5.5.2")                 ;A few commits ahead, really. To fix some bugs.

Please use the versioning scheme for Git snapshots explained here:

  https://www.gnu.org/software/guix/manual/en/html_node/Version-Numbers.html

> +                 `("ARCAN_SCRIPTPATH" ":" suffix
> +                   (,(string-append out "/share/arcan/scripts")))))

[...]

> +    (native-search-paths
> +     (list (search-path-specification
> +            (variable "ARCAN_APPLBASEPATH")
> +            (separator #f)
> +            (files '("share/arcan/appl")))))

Should ARCAN_SCRIPTPATH also be here?

> +       ("arcan-openal" ,(origin
> +                          (method git-fetch)
> +                          (uri (git-reference
> +                                (url "https://github.com/letoram/openal.git")
> +                                (commit "1c7302c580964fee9ee9e1d89ff56d24f934bdef")))
> +                          (sha256
> +                           (base32
> +                            "0dcxcnqjkyyqdr2yk84mprvkncy5g172kfs6vc4zrkklsbkr8yi2"))))))

Did you try vanilla OpenAL instead?  If you have a pointer to a page
explaining why this variant is needed, that’d be great!  Also please add
a ‘file-name’ for this ‘origin’.

> +    (native-inputs
> +     `(("pkg-config" ,pkg-config)
> +       ("ruby" ,ruby)))                 ; For documentation and testing
> +    (home-page "https://arcan-fe.com")
> +    (synopsis "Combined display server, multimedia framework and game engine (egl-dri)")

Maybe just “Multimedia framework and game engine”?

> +    (description "Arcan is a powerful development framework for creating virtually
> +anything from user interfaces for specialized embedded applications
> +all the way to full-blown desktop environments")

Missing period at the end, “powerful” is maybe not factual enough, and
it would be good if you could expound with just a sentence or two (see
<https://www.gnu.org/software/guix/manual/en/html_node/Synopses-and-Descriptions.html>).

> +    (license (list license:gpl2+
> +                   license:lgpl2.0
> +                   license:bsd-3))))

Could you add a comment above explaining the meaning of the list
(whether it’s triple-licensed or what)?

Apart from that this LGTM.  It’s great that you managed to unbundle
almost everything.

Could you send an updated patch?

Thanks!

Ludo’.