guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#57540] [PATCH] Add ocaml-elpi (a dependency of coq-mathcomp-analysi


From: Julien Lepiller
Subject: [bug#57540] [PATCH] Add ocaml-elpi (a dependency of coq-mathcomp-analysis)
Date: Sat, 3 Sep 2022 20:40:35 +0200

Hi Garek,

thanks for the patches! As I understand it, these patches introduce 8
new packages, but I count only 7 (with this one which didn't make it as
a part of the series). Could you split the last patch, so each new
package has its own patch?

Also, some points in your patches:

For "Don't know about directory test", you can try with an argument:
#:test-target "some-dir". This directory is the directory where tests
are located. If there is no single directory, you can try
#:test-target "."

Descriptions should use our subset of texinfo. Basically, lists should
look like:

@itemize
@item foo
@item bar
@end itemize

and you can use @file, @code, etc as you see fit (to replace markdown's
"`").

Whenever possible, we try not to rely on github tarballs, so it'd be
best to use a git-reference in all your patches :)

In coq-elpi:

> +                      ;; Remove the useless line
> +                      ;; "src/META.coq-elpi"
> +                      ;; in file _CoqProject.
> +                      ;; It does not affect
> +                      ;; the success of compliation.

should be "compilation". And then I wonder why you need that at all, if
it doesn't change compilation result?

> +                      #t))

Please don't use #t at the end of phases anymore.

> +                  (replace 'check

The gnu-build-system already runs "make test", doesn't it? Why do you
need to replace this phase?


In coq-mathcomp-hierarchy-builder:

> +                  (replace 'check

Instead of replacing this phase, maybe try #:test-target "test-suite"

> +       ;; Makefile.common has no references to tests at all
> +       ;; (yet).

If that means that future versions will have tests (for instance if
master has tests), maybe this should say it more explicitely.

Once you fix all this, could you send a v2 series? Thank you!





reply via email to

[Prev in Thread] Current Thread [Next in Thread]