Skip to content

Conversation

art-w
Copy link
Collaborator

@art-w art-w commented Mar 20, 2025

  • Added conf-libmongoc.opam to use opam depexts for the dependency: Sadly I could not find libmongoc for all OS... Enabling ocaml-ci would help verify that the other package names are correct because I could not test them as github CI only provides ubuntu/mac/windows (but AFAIK the repo needs to be public first for ocaml-ci).
  • The test omont.t assumes that an instance of MongoDB is running and that the mongosh binary is installed... This setup was easy to reproduce for ubuntu, but for macos I only test if it builds. This will cause some issues when publishing with the opam CI since mongo will not be runnable easily (and ocaml-ci will definitively not be happy)
  • Finally the documentation is automatically available at https://tarides.github.io/mongoc-ocaml/mongoc/Mongoc/index.html :)

@cuihtlauac
Copy link
Member

but AFAIK the repo needs to be public first for ocaml-ci

The repo is already public, @samoht has changed the settings earlier this week.

The test omont.t assumes that an instance of MongoDB is running and that the mongosh binary is installed... This setup was easy to reproduce for ubuntu, but for macos I only test if it builds

Overall I'm unsure of what to test, under which systems and hypothesis regarding a binding.

I wrote this cram test to enable comprehensive changes without worrying too much. I'm unsure if it's the right approach in the long run.

Copy link
Member

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

Thanks @art-w, that looks good to me

@cuihtlauac cuihtlauac merged commit f2b6727 into main Mar 20, 2025
2 checks passed
@art-w
Copy link
Collaborator Author

art-w commented Mar 20, 2025

Oh right it's public thanks! I enabled ocaml-ci on the repo but we should expect some issues with the test and the depexts

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