Skip to content
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

3 packages from ngernest/mica at 0.1.0 #26652

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ngernest
Copy link

@ngernest ngernest commented Oct 1, 2024

This pull-request concerns:

  • ppx_mica.0.1.0: PPX deriver that automates differential testing for OCaml modules (OCaml Workshop '24 paper)
  • mica_case_studies.0.1.0: Mica case studies (part of the OCaml Workshop '24 artifact)
  • tyche_utils.0.1.0: Utilities for the Tyche VS Code extension to visualize results produced from tests run by Mica


🐫 Pull-request generated by opam-publish v2.4.0

@shonfeder
Copy link
Collaborator

shonfeder commented Oct 2, 2024

Hello! Thank you for publishing your package! Mica looks super cool and I am looking forward to trying it out. (The readme is extremely clear!)

Since it looks like this is your first time publishing a package to opam, allow me offer a warm welcome and a bit of an intro :)

We run a CI system to ensure that packages can be installed on all intended systems. You can see the results for your PR at https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/51b56fad1033fa4ad04b706eef5074af6f223397 (or by clicking the Details link next to the opam-ci status check).

This shows the results of the builds and tests across our build matrix. Because we build and test on such a large matrix, it is very common for errors to be identified during package publication.

In my following comments and suggestions, I'll try to provide pointers and suggestions to get everything squared away. Please let us know if you have any questions, as we are here to help :)

Copy link
Collaborator

@shonfeder shonfeder left a comment

Choose a reason for hiding this comment

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

I think this should address all the errors I am seeing!

bug-reports: "https://github.com/ngernest/mica/issues"
depends: [
"dune" {>= "3.7"}
"yojson"
Copy link
Collaborator

Choose a reason for hiding this comment

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

One of the errors is a "lower bounds check failure". The actual errors looks like this:

#=== ERROR while compiling tyche_utils.0.1.0 ==================================#
# context              2.3.0~alpha~dev | linux/x86_64 | ocaml-base-compiler.4.11.2 | pinned(https://github.com/ngernest/mica/archive/refs/tags/v0.1.0.tar.gz)
# path                 ~/.opam/4.11/.opam-switch/build/tyche_utils.0.1.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p tyche_utils -j 39 @install
# exit-code            1
# env-file             ~/.opam/log/tyche_utils-7-a31302.env
# output-file          ~/.opam/log/tyche_utils-7-a31302.out
### output ###
# File "/home/opam/.opam/4.11/lib/ocaml-compiler-libs/bytecomp/ocaml-compiler-libs.bytecomp.dune", line 1, characters 0-0:
<snip>
# File "lib/tyche_utils/tyche_utils.ml", line 18, characters 13-27:
# 18 |   val json : Yojson.Basic.t
#                   ^^^^^^^^^^^^^^
# Error: Unbound type constructor Yojson.Basic.t

This is taken from the CI log https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/51b56fad1033fa4ad04b706eef5074af6f223397/variant/compilers,4.11,tyche_utils.0.1.0,lower-bounds

It indicates that the package depends on a feature of Yojson that is not available in some versions matching the dependency specification.

To fix this, we can add a lower bound. This is probably a safe bet, given the change log:

Suggested change
"yojson"
"yojson" {>= "2.0.O"}

"yojson"
"base" {>= "v0.15.1"}
"core_unix" {>= "v0.15.0"}
"odoc" {with-doc}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have tests (failed: Library "alcotest" not found.)

#=== ERROR while compiling tyche_utils.0.1.0 ==================================#
# context              2.3.0~alpha~dev | linux/x86_64 | ocaml-base-compiler.4.11.2 | pinned(https://github.com/ngernest/mica/archive/refs/tags/v0.1.0.tar.gz)
# path                 ~/.opam/4.11/.opam-switch/build/tyche_utils.0.1.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p tyche_utils -j 39 @install @runtest
# exit-code            1
# env-file             ~/.opam/log/tyche_utils-7-ad7cb0.env
# output-file          ~/.opam/log/tyche_utils-7-ad7cb0.out
### output ###
# File "test/utils_test/dune", line 3, characters 21-29:
# 3 |  (libraries ppx_mica alcotest ppxlib)
#                          ^^^^^^^^
# Error: Library "alcotest" not found.
# -> required by _build/default/test/utils_test/test_runner.exe
# -> required by alias test/utils_test/runtest in test/utils_test/dune:2

Which is just indicating that the package doesn't declare the needed test dependency on alcotest:

Suggested change
"odoc" {with-doc}
"odoc" {with-doc}
"alcotest" {with-test}

@@ -0,0 +1,45 @@
# This file is generated by dune, edit dune-project instead
opam-version: "2.0"
synopsis: "Mica case studies (for OCaml Workshop 2024 talk)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this package expected to be of value for end-users? If it is only/primarily meant as an example or for test cases, please remove it from the publication, as per our policy that Packages should provide some sort of utility for users. However, if it is of some general utility, please add a description to explain.

"ppx_jane" {>= "v0.15.0"}
"ppx_assert" {>= "v0.15.0"}
"base" {>= "v0.15.1"}
"base_quickcheck" {>= "v0.15.0"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

ppx_mica.0.1.0 (failed: Library "tyche_utils" not found.) tells us that we are missing the dependency on the tyche_utils here:

Suggested change
"base_quickcheck" {>= "v0.15.0"}
"base_quickcheck" {>= "v0.15.0"}
"tyche_utils" {=version}

This uses the version package variable to ensure the versions stay in sync. If you expect the API of the utils to be stable accross versions, you could probably omit this constraint.

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

Successfully merging this pull request may close these issues.

2 participants