Skip to content

Conversation

@ksromanov
Copy link
Contributor

@ksromanov ksromanov commented May 2, 2020

Thank you very much for your repo - I am going to use it for benchmarking TRMC. Since I need to convert it to dune anyway, I am making a PR to update your repo.

No functionality changes done, only:

  1. s/jbuilder/dune/ + a small updates of command line arguments to dune
  2. Ran dune upgrade as advised by dune.
  3. Updated .opam to pass opam lint for opam version 2.1.0~beta.

Please feel free to close this PR without comments and/or merging.

Copy link
Owner

@aantron aantron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just have a couple nits.

Makefile Outdated
.PHONY : run
run :
jbuilder build --dev tester/tester.exe reporter/reporter.exe
dune build --profile=dev tester/tester.exe reporter/reporter.exe
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think --profile=dev could just be dropped, since it is the default.

faster-map.opam Outdated
opam-version: "1.2"
opam-version: "2.0"
synopsis: "A tail-recursive list map with good performance for all list sizes. Not actually written in assembly"
authors: "<Anton Bachin> [email protected]"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this is should be name <email>, not <name> email.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

faster-map.opam Outdated
"containers"
"core_bench"
"jbuilder" {build & >= "1.0+beta13"}
"dune" {build & >= "1.0"}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dune should be a non-build dependency. See ocaml/opam-repository#14266.

@aantron aantron merged commit b617204 into aantron:master May 2, 2020
@aantron
Copy link
Owner

aantron commented May 2, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants