Subject: Re: [PATCH v2] gnu: bqn: Add bqn.scm and dbqn package.

From: Christopher Rodriguez <[email-masked]>
Date: Fri, 05 Aug 2022 11:12:53 -0400

Liliana Marie Prikler <[email-masked]> writes:

> merge 56989 56990 56991 56992 56993
> thanks

Is this notation something anyone can do? I would very much like to be
able to fix my own mistakes in the future.

> Regarding the patch title, just one level of grouping is enough.
> That is, use "gnu: Add dbqn."
>
> Also, you're missing a ChangeLog, i.e.
>
> * gnu/packages/bqn.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Register it here.

I have amended the commit message to reflect the above. Thanks for the
tips!

I've also added `%D%/packages/bqn.scm` to `gnu/local.mk`, as requested.

> Don't let-bind tag, version and hash, use them inline.
...
> Note that version will be bound here even if you use the version field
> to do so.

This makes sense, and I've removed the let binding entirely. My only
uncertainty is where "revision" should go; I've currently attached it to
the upstream version tag (version "0.2.1-1"), where "0.2.1" is the tag
and "1" is the revision. Is this correct?

> You could do
>   (replace 'check
>     (lambda* (#:key tests? #:allow-other-keys)
>       (when tests?
>         (for-each (lambda (known-good-test)
>                     (invoke my-glorious-bin known-good-test))
>                   known-good-tests))))
> FSVO my-glorious-bin and known-good-tests.

I plan to do this once I've been able to look at each test and the
entire source and see if I can get it working. I've added an issue
upstream[1] where the author of the package has confirmed it is on "just
enough life support" to build the recommended implementation from
source.

As it stands, I would have to test each test individually anyway, and
only add it to the package if it arbitrarily passes on my machine for
some reason. I don't think there is value there, as tests are meant to
ensure consistency and I cannot do that using such a workflow.

And though this *is* and *should be* a public package, it is *not* the
recommended interpreter for the language. It is primarily included here
to build the recommended one (CBQN) from source, along with some other
tools I've yet to package that require it during build.

> Could this be done in/before install?

It could, in fact. I've moved it to the above step, and deleted subjars
entirely.

> You can use #:rest args to bind args for apply.  Also use assoc-ref
> rather than cdr + assoc.

I had, for some reason, flipped the arguments on assoc-ref (which
obviously didn't work) and when that failed fell back to cdr + assoc. I
woke up this morning and noticed my mistake; It is fixed now.

As for the #:rest args recommendation: I cannot figure out how to
explicitly bind (list options) to #:options in the apply call using
#:rest. This is probably ignorance on my part; I am still learning the
some of the mechanisms in scheme, and have not used #:rest (or the dot
notation for it) much at all.

Is there an example You could point me to so I can educate myself?

> Is OpenJDK 17 required?

Really, only a JDK 7+ is required. openjdk17 carries the "openjdk" label
currently, and so I defaulted to that one. Is there another I should use
in my packages instead?

> Cheers

Thank You for the speedy response!

--

Christopher Rodriguez