Subject: Re: [bug#38596] [PATCH] gnu: Add PureScript.

From: Christopher Baines <[email-masked]>
Date: Tue, 21 Apr 2020 08:55:37 +0100

John Soo <[email-masked]> writes:

> Christopher Baines <[email-masked]> writes:
>
>> I've had an initial look through these now, sorry it's taken so long!
>
> No problem. I was thinking I ought to do some qa and review, myself.
>
>> ghc-utf8-light could do with a better description, something a little
>> more informative.
>
> I think I made this better as well as fixing some other descriptions
> lacking full sentences.

Great :)

>> The ghc-sourcemap and ghc-mtl-compat tests are disabled, it would be
>> good to at least add a comments as to why.
>
> I added a comment for sourcemap. I realized mtl-compat's tests and
> haddocks worked just fine so those are re-enabled.

Cool.

>> The ghc-protolude synopsis could potentially be more informative.
>
> I think I added some more commentary there. It's a bit tricky since I
> have not used protolude myself so I am not sure what to provide aside
> from their stated description.

Sure, I think it's fine now.

Some really minor description things I missed earlier:

ghc-aeson-better-errors, ghc-transformers and ghc-websockets have URLs
in the description, @url could probably be used.

I'd also remove the following from the ghc-bower-json description, it
reads more like uninformative advertising to me and isn't really
relevant to this specific package:

  Bower is a package manager for the web (see http://bower.io).

>> I think adding specific versions of dependencies for purescript is OK,
>> but only if they're justified, like if the package only builds with that
>> version. The justification should also be noted in the purescript
>> package description.
>
> Ah, thanks. Would a comment above the dependencies suffice? The packages
> won't be searchable since they are not define-public (yet?).

Maybe it would be good to note why the package in Guix didn't work, like
I tried to use [email-masked], and the build phase failed with some
type errors. That'll at least point out clearly that there's an
incompatability. I don't think much detail is needed, the following
would be fine.

     `(("ghc-happy" ,ghc-happy-1.19.9)  ; build fails with 1.19.12

>> I don't think it's necessary to go through all the custom versions of
>> packages you've added, but it would be good to at least see if there's
>> one case where the upstream specification is overly strict, and the
>> slightly different version from Guix would work. That at least would
>> provide an example of how to override versions going forward.
>
> Ah thanks for the suggestion here. I found one that could be taken from
> the existing package in (gnu packages haskell-xyz). The rest seemed to
> be either pinned exactly or too strict to take from guix packages.

Thanks, although my intent was more about adding something like this.

  #:phases
  (modify-phases %standard-phases
    (add-after 'unpack 'patch
      (lambda _
        (substitute* "purescript.cabal"
          (("clock .*$")
           "clock,\n"))
        #t)))))

Now I'm not sure quite how risky this is, the purescript tests aren't
being run, but given this is Haskell and it compiles, hopefully that
provides some guarantees.

What do you think?

Thanks,

Chris