On Fri, Sep 10 2021, Vivien Kraus via Guix-patches via wrote:

> Hello Maxime,
>
> Le vendredi 10 septembre 2021 à 12:25 +0200, Maxime Devos a écrit :
>> minetest-advtrains and minetest-basic-trains are separate packages
>> that
>> can be added separately, so theoretically, they should be added in
>> separate
>> patches:
> It seems like the least surprising thing to do, so let’s go with the
> split patch.
>
>> Maybe replace ‘realistic trains’ -> ‘realistic train system’,
> I’m not sure. Upstream really likes to use "train" as a general term,
> like in share/minetest/mods/advtrains/modpack.conf, the official
> description, the wiki, ... so I think it’s better to use that.
>
>> and note that minetest-basic-trains contains the actual train models.
> This sentence was missing, and I added it.
>
>> I think a linter will have something to say about long lines here
>> (try "./pre-inst-env guix lint minetest-advtrains minetest-basic-
>> trains").
> You’re right, I fixed the long lines and other warnings. However, I
> don’t know how to fix "no updater for minetest-xxx" for both minetest-
> advtrains and minetest-basic-trains. I can’t see a meaningful
> difference between my packages and the others.
>
> Vivien
> From fbe39d93acb01fbef9f7f497e9bb5814fdf3f6b5 Mon Sep 17 00:00:00 2001
> From: Vivien Kraus <vivien@planete-kraus.eu>
> Date: Fri, 10 Sep 2021 14:21:47 +0200
> Subject: [PATCH 4/4] gnu: add minetest-basic-trains
>
> * gnu/packages/minetest.scm (minetest-basic-trains): New variable.
> ---
>  gnu/packages/minetest.scm | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/gnu/packages/minetest.scm b/gnu/packages/minetest.scm
> index 0b0e7ace96..c218fb827f 100644
> --- a/gnu/packages/minetest.scm
> +++ b/gnu/packages/minetest.scm
> @@ -668,3 +668,35 @@ stopping before signals.
>       (list license:cc-by-sa3.0 license:agpl3+))
>      (properties
>       `((upstream-name . "orwell/advtrains")))))
> +
> +(define-public minetest-basic-trains
> +  (package
> +    (name "minetest-basic-trains")
> +    (version "1.0.1")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "http://git.bananach.space/basic_trains.git/")
> +             (commit
> +              "d44c410f7c2a7202ee68b66fc50febae89e0c5dc")))
> +       (sha256
> +        (base32
> +         "0vvzndj48kgdz2bfgivfm217sbmc2lmxpp2mispcy7byn4i26prx"))
> +       (file-name (git-file-name name version))))
> +    (build-system minetest-mod-build-system)
> +    (propagated-inputs
> +     `(("minetest-advtrains" ,minetest-advtrains)))
> +    (home-page
> +     "http://advtrains.de/wiki/doku.php?id=usage:trains:basic_trains")
> +    (synopsis
> +     "\
> +Collection of basic trains for the Advanced Trains mod")
> +    (description
> +     "\
> +This modpack contains the trains which were the \"default\" trains in
> +advtrains up to version 2.2.1.")

I would use ``default'', which is the correct Texinfo syntax,
instead of "default".  See the Texinfo manual for more details:

  <https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Inserting-Quotation-Marks.html>

> +    (license
> +     (list license:cc-by-sa3.0 license:agpl3+))
> +    (properties
> +     `((upstream-name . "orwell/basic_trains")))))
> -- 
> 2.33.0
>
> From 9675601acba125905877f30b5bc47053db3b652d Mon Sep 17 00:00:00 2001
> From: Vivien Kraus <vivien@planete-kraus.eu>
> Date: Sun, 5 Sep 2021 15:21:35 +0200
> Subject: [PATCH 3/4] gnu: add minetest-advtrains
>
> * gnu/packages/minetest.scm (minetest-advtrains): New variable.
> ---
>  gnu/packages/minetest.scm | 44 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/gnu/packages/minetest.scm b/gnu/packages/minetest.scm
> index 9e48d5c69c..0b0e7ace96 100644
> --- a/gnu/packages/minetest.scm
> +++ b/gnu/packages/minetest.scm
> @@ -624,3 +624,47 @@ track of important locations.")
>                     license:cc-by4.0 license:cc-by-sa3.0 license:public-domain
>                     license:cc0 license:fdl1.2+))
>      (properties `((upstream-name . "RealBadAngel/unified_inventory")))))
> +
> +(define-public minetest-advtrains
> +  (package
> +    (name "minetest-advtrains")
> +    (version "2.3.1")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://git.bananach.space/advtrains.git")
> +             (commit "release-2.3.1")))

This would preferably be (commit (string-append "release-" version)),
that means that you only have to update the ‘version’ field and the hash
when updating the package (assuming it doesn’t need additional
dependencies or change build system, etc.).

Otherwise, LGTM!