New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
elpi: 1.4.1 -> 1.6.0, and coq-elpi #66183
Conversation
bc25ebb
to
e731156
Compare
@GrahamcOfBorg build coqPackages_8_10.coq-elpi |
in | ||
|
||
stdenv.mkDerivation rec { | ||
name = "coq${coq.coq-version}-elpi-${param.version}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this is more right, but I think this is "more correct"
name = "coq${coq.coq-version}-elpi-${param.version}"; | |
name = "coq-${coq.coq-version}-${coq.ocamlPackages.elpi.name}"; |
"coq8.10-elpi-master" as name to "coq-8.10-elpi-1.6.0"
however, since you're taking depending on a specific commit, the normal convention is
name = "coq${coq.coq-version}-elpi-${param.version}"; | |
pname = "coq-elpi"; | |
version = "unstable-2019-08-02"; |
I don't think i've seen a package that had dependencies as part of the title.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly personal preference.
However, you should separate out the maintainer entry, the bump, and coq-elpi in their own commits.
maintainers: add cohencyril
elpi: 1.4.1 -> 1.6.0
coq-elpi: init at unstable-2019-08-02
|
||
installFlags = "COQLIB=$(out)/lib/coq/${coq.coq-version}/"; | ||
|
||
meta = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meta = { | |
meta = with stdenv.lib; { |
|
||
meta = { | ||
description = "Coq plugin embedding ELPI."; | ||
maintainers = [ stdenv.lib.maintainers.cohencyril ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maintainers = [ stdenv.lib.maintainers.cohencyril ]; | |
maintainers = with maintainers; [ cohencyril ]; |
meta = { | ||
description = "Coq plugin embedding ELPI."; | ||
maintainers = [ stdenv.lib.maintainers.cohencyril ]; | ||
license = stdenv.lib.licenses.lgpl21; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
license = stdenv.lib.licenses.lgpl21; | |
license = licenses.lgpl21; |
nativeBuildInputs = [ which ]; | ||
buildInputs = [ coq coq.ocaml ] ++ (with coq.ocamlPackages; [ findlib elpi ]); | ||
|
||
installFlags = "COQLIB=$(out)/lib/coq/${coq.coq-version}/"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
installFlags = "COQLIB=$(out)/lib/coq/${coq.coq-version}/"; | |
installFlags = "COQLIB=${placeholder "out"}/lib/coq/${coq.coq-version}/"; |
@GrahamcOfBorg build ocaml-ng.ocamlPackages_4_04.elpi |
Motivation for this change
update of elpi and adding compatible development version of coq-elpi
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @vbgl