Subject: [bug#36352] [PATCH] gnu: Add solvespace.

From: Ludovic Courtès <[email-masked]>
Date: Wed, 03 Jul 2019 00:30:52 +0200

Hello Myles,

Myles English <[email-masked]> skribis:

> * gnu/packages/game-development.scm (solvespace): New variable and
> dependencies on gnu package modules.

The patch LGTM overall.  Here are some comments/questions:

> +(define-public solvespace-3
> +  (let ((commit "5df53fc59e7f31e265fabd4c15e6601bd3032833")
> +	(revision "1"))
> +    (package
> +      (name "solvespace")
> +      (version (git-version "3.0" revision commit))

Why choose this commit specifically?  ‘git describe’ returns
“v2.1.rc1-570-g5df53fc”, and in fact there’s no “v3.0” tag, so this
version string is a bit misleading.

The general policy is to use the latest release, but if there’s a
compelling argument, we can use another commit; in that case, it’s
better to have a comment explaining the choice.

> +         (uri (git-reference
> +               (url "https://github.com/solvespace/solvespace.git")
> +               (commit commit)
> +               (recursive? #t)))

Is ‘recursive?’ needed?  If it’s just for the bundled dependencies under
extlib/ that are not used anyway, perhaps we can omit it?

> +      (synopsis "Parametric 2D/3D CAD tool")
> +      (description "Parametric 2D/3D computer aided design (CAD) tool and
> +constraint-based parametric modeler with simple mechanical simulation
> +capabilities.")

Could you make it a full sentence and perhaps expound a little bit, as
per <https://gnu.org/s/guix/manual/en/html_node/Synopses-and-Descriptions.html>?

Thank you, and apologies for the delay!

Ludo’.