mbox series

[bug#69552,0/2] Add cyclone scheme

Message ID cover.1709580311.git.takev@disroot.org
Headers
Series Add cyclone scheme |

Message

TakeV March 4, 2024, 7:29 p.m. UTC 
I saw someone wished for cyclone scheme to be added to the guix package repo. It is a R7RS scheme which compiles to c. Seems pretty neat!

TakeV (2):
  gnu: Add cyclone-bootstrap.
  gnu: Add cyclone.

 gnu/packages/scheme.scm | 57 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)


base-commit: ff3c592710120de2ac05972a85892cdf4b1db101

Comments

Juliana Sims March 4, 2024, 10:29 p.m. UTC | #1 
Hello,

Thanks for this patch! It's always good to get more Scheme into Guix :)

 From a purely technical perspective, there's nothing wrong with these 
patches. They build fine, are reproducible, and seem well-enough-styled 
for all practical purposes. Make sure to run `guix lint` over the 
patches; I get a couple warnings when I do that locally.

As a sidenote, Cyclone fails to build on a supported architecture 
(aarch64) because the `ck` package fails to build there. Interestingly, 
it builds fine on 32-bit ARM (armhf). I note this merely for posterity 
and anyone interested in fixing `ck`. Everything builds just fine on 
x86(_64).

What follows is largely stylistic feedback.

First patch:

 > ;; cyclone is self-hosted. To build it, we require the bootstrapped 
compiler.

I would capitalize "Cyclone" here and elsewhere; it seems to be 
capitalized in the project's prose about it. Also, this is the 
"bootstrap" compiler -- the "bootstrapped" compiler would be the 
compiler compiled with this one.

 >     (synopsis "Install Cyclone Scheme on your machine.")

This is inaccurate as this package does not install Cyclone Scheme on 
the user's machine. Perhaps simply "Cyclone Scheme bootstrap compiler" 
or "Bootstrap Cyclone Scheme" would work? Synopses are, notably, not 
meant to be complete sentences and thus should not have final 
punctuation.

 >     (description "Bootstrap the generate the cyclone scheme 
compiler")

This description is a bit short. I would just combine the first two 
paragraphs of the project README into one with minor edits to get the 
following description string:

"Cyclone Scheme is a brand-new, R7RS Scheme-to-C compiler that uses a 
variant of Cheney on the MTA to implement full tail recursion, 
continuations, and generational garbage collection.  This package uses 
intermediate code generated by compiling the Scheme source files to 
build and install Cyclone Scheme.  The compiler is self-hosting and 
cannot be built directly on a system without Cyclone binaries 
installed."

Strictly speaking, "brand new" does not need a hyphen, but that's the 
way the project writes it so I've left it as-is. You probably also want 
to add a linebreak after `description` so the string starts on its own 
line; imo this makes the line alignment more esthetically pleasing when 
complying with the 80-column width limit.

Second patch:

 >     ;; the bootstrapped compiler and final compiler share most build 
reqs

"bootstrapped" -> "bootstrap"

 >     (synopsis "Fast R7RS scheme which compiles to C")

Capitalize "Scheme." This could also just be "Fast R7RS Scheme-to-C 
compiler" if you want.

 >     (description
 > "A brand-new compiler that allows practical application
 > development using R7RS Scheme. We provide modern features and a 
stable
 > system capable of generating fast native binaries.")

I would restore the README's full "Cyclone Scheme is a" at the 
beginning of this; the description can refer to the package in the 
third person. Also, first-person language should probably be avoided 
here. "We" in a Guix package description would most directly imply the 
Guix project; this is inaccurate. Perhaps replace "We provide" with 
"Cyclone provides." It may also be fruitful to cut the second sentence 
entirely and enumerate some of the compiler's said features. I would 
try to modify the language of the second paragraph in the project's 
README (replacing passive voice with active voice, for example), and/or 
add a list highlighting some (all would be a bit too long I think) of 
the "Features" section of the aforementioned README. They seem very 
proud of the "Cheney on the MTA" algorithm so make sure to highlight it.

Other than those stylistic notes, this patch series looks good to me!

Thanks,
Juli
Juliana Sims March 7, 2024, 10:20 p.m. UTC | #2 
LGTM!