search

ocurrent / ocaml-ci

A CI for OCaml projects
https://ocaml.ci.dev
112 stars 74 forks source link

Missing `.ocamlformat` version line causes analysis failure #893

Closed benmandrew closed 9 months ago

benmandrew commented 11 months ago

Issue opened from comment by @gpetiot on issue #834, as the technical background is slightly different:

We still observe this on ocamlformat's repository: https://github.com/ocaml-ppx/ocamlformat/runs/17517278251 (we don't have a set version in the .ocamlformat file because we use the version of the same commit to format ocamlformat's codebase)

I do wonder where the line is between supporting special use-cases and missing actual errors that should be surfaced in a normal use-case. You could imagine someone with an accidentally malformed .ocamlformat file wondering why it's not being tested.

@tmcgilchrist WDYT?

Julow commented 10 months ago

The main issue is that an error while interpreting the .ocamlformat file makes the analysis job fail, for example https://github.com/ocaml-ppx/ocamlformat/pull/2371/checks?check_run_id=18434405707

The detected error should instead fail the fmt job.

There's also a second issue: Local OCamlformat package no longer supported.

If the version is unspecified, the CI wouldn't be able to install ocamlformat. Though, the repository might contain OCamlformat (eg. in a duniverse), in which case ocamlformat is not a dependency of the fmt job. This used to be supported by the CI.

You could imagine someone with an accidentally malformed .ocamlformat file wondering why it's not being tested.

I suggest spawning the fmt job but making it fail immediately with an informative error message.