-
Notifications
You must be signed in to change notification settings - Fork 6
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
Rename and fix unit test target #47
base: master
Are you sure you want to change the base?
Conversation
35cf5d4
to
701462d
Compare
a9e9dce
to
d24f718
Compare
@@ -35,10 +35,10 @@ jobs: | |||
with: | |||
ocaml-version: ${{ matrix.ocaml-version }} | |||
- name: Install dependencies | |||
if: steps.cache-opam.outputs.cache-hit != 'true' |
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.
@mikonieminen @joseferben This line prevents alcotest from being installed, so I deleted it. Is there another way to circumvent that?
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.
I usually install in manually in a prior step.
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.
Where would you put it, do you have an example?
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.
Here we install some system deps before everything else: https://github.com/oxidizing/sihl/blob/master/.github/workflows/ci.yml#L43
And then we run make deps
which install a bunch of dev dependencies.
.PHONY: deps
deps:
opam install -y dune-release merlin ocamlformat utop
opam install -y alcotest-lwt mariadb.1.1.4 caqti-driver-postgresql.1.8.0 caqti-driver-mariadb.1.8.0
opam install . -y --deps-only --locked
eval $(opam env)
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.
Looking at the CI file, I think you can update to uses: avsm/setup-ocaml@v2
and then omit those anyway.
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.
- name: Use OCaml ${{ matrix.ocaml-version }}
uses: ocaml/setup-ocaml@v2
with:
ocaml-compiler: ${{ matrix.ocaml-compiler }}
dune-cache: true
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.
This does not seem to work, I think we still need to install dependencies somewhere? It's saying ca-certs
is missing now
#46