GNU bug report logs - #36352
[PATCH] gnu: Add solvespace.

Previous Next

Full log

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Myles English <mylesenglish <at> gmail.com>
Cc: 36352 <at> debbugs.gnu.org, Myles English <myles <at> tdma.co>
Subject: Re: [bug#36352] [PATCH] gnu: Add solvespace.
Date: Wed, 03 Jul 2019 00:30:52 +0200

Hello Myles,

Myles English <mylesenglish <at> gmail.com> 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’.

Previous Next

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