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

chore: enable ocaml trunk job on ci #7055

Merged
merged 3 commits into from
Mar 21, 2023

Conversation

Alizter
Copy link
Collaborator

@Alizter Alizter commented Feb 13, 2023

This will have to be bumped with every new OCaml release, but AFAICT there is not way to generically rely on trunk, since the OCaml action works on the latest opam repo which doesn't have an entry for ocaml trunk without a version.

Should I make these jobs allow fail? I guess if the compiler team moves libraries around, this has a chance of breaking.

close #2298

<!-- ps-id: 686589c7-7eff-4313-b0eb-d8e1a7429d70 -->

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter requested a review from rgrinberg February 13, 2023 02:08
@emillon
Copy link
Collaborator

emillon commented Feb 13, 2023

One rule I like is that we should test something moving against something stable - for example dune main vs ocaml release, or latest dune release vs ocaml trunk - otherwise when something breaks it's difficult to determine who's at fault.

I don't think there's much value in adding that job. We're already proactive in adding canary tests for the next version of the compiler and adopting it in our tests. I'm not sure what kind of issues we would find with that extra job.

@rgrinberg
Copy link
Member

We're already proactive in adding canary tests for the next version of the compiler and adopting it in our tests.

Where can I see our canary tests?

@emillon
Copy link
Collaborator

emillon commented Feb 14, 2023

Since several releases, there's a readiness process put in place by the compiler team to make sure that some core tools including dune are compatible with it. It's not automated but I don't think it needs to be.
That being said, that's a change that's easy to revert so it's not a big risk either.

@Alizter
Copy link
Collaborator Author

Alizter commented Mar 12, 2023

What do we do about this then?

@rgrinberg
Copy link
Member

I think it's useful. Yes, it's difficult to determine who is at fault, but at least we find out there is a fault right away and not months after the offending commit was merged.

I'll leave the decision to @Alizter. If he finds it useful, then we can merge. I would personally run at least the minimal set of the test suite that doesn't rely on any external dependencies as well though

@Alizter
Copy link
Collaborator Author

Alizter commented Mar 20, 2023

Let's keep it around for now.

I don't know how to run the test suite for just tests without external deps (we should add a makefile target for that, not sure how).

@rgrinberg can you merge when the CI is happy?

@Alizter Alizter merged commit d3e9b73 into ocaml:main Mar 21, 2023
@Alizter Alizter deleted the ps/rr/chore__enable_ocaml_trunk_job_on_ci branch March 21, 2023 20:53
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.

[ci] Testing the build with OCaml trunk
3 participants