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

[Myles English <mylesenglish@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2783 bytes --]

Hi Ludo',

Thanks a lot for your comments, sorry I hadn't replied yet.

On Mon, 2 Sep 2019, 14:24 Ludovic Courtès, <ludo@gnu.org> wrote:

> Did you have a chance to look into that?  (See below.)
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
> > Hello Myles,
> >
> > Myles English <mylesenglish@gmail.com> skribis:
> >
> >> * gnu/packages/game-development.scm (solvespace): New variable and
>

game-development.scm? I think I put it in the wrong place.

>> 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.
>

It looks as though a longawaited v3.0 release may be imminent, hence my
delay in replying to your advice.  I'll wait another week or two before
asking if it is going to drop 'soon'.  (I would have tried to justify my
choice by referencing the unofficial Debian package, mentioned on the
project's github page, using the master branch and calling itself v3.0.)

>> +         (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?
>

Some of them are still used (sorry I can't check which ones just now).  I
have trivially modified the build system so that if :recursive? is a list
it only clones those submodules listed. I can find the patch later but its
just a couple of lines.  If this is adopted should may be rename
:recursive? or add another parameter?

>> +      (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
> >?
>

I'll try and improve this when I make a patch for the true v3.0.

Myles

>

[-- Attachment #2: Type: text/html, Size: 5515 bytes --]