Skip to content

CP-307947: [maintenance]: add .mli files to all database modules #6460

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

Merged
merged 2 commits into from
May 9, 2025

Conversation

edwintorok
Copy link
Contributor

ocaml-lsp-server's 'Action->Infer interface' was used for this, and then removed internal/unused entries.

Having an mli file is useful:

  • can find dead code more easily if it is clear which functions are internal to a module and which aren't
  • the impact of refactoring changes is more obvious (did we have to change an mli at all?)
  • makes it easier to understand what a module does

There are some .ml files which only contain types, these are instead renamed to .mli and dune's modules_without_implementation feature is used.
Executables get an empty .mli (recent versions of dune already do the equivalent of this, but this makes it more obvious).

Drop code from .ml files that now show up as unused.

Eventually we can also add some documentation, this will be done in followup PRs for new code.

edwintorok added 2 commits May 9, 2025 09:15
Used 'dune format-dune-file dune >dune && mv dune.tmp dune'

No functional change.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
ocaml-lsp-server's 'Action->Infer interface' was used for this,
and then removed internal/unused entries.

Having an mli file is useful:
* can find dead code more easily if it is clear which functions are internal to a module and which aren't
* the impact of refactoring changes is more obvious (did we have to change an mli at all?)
* makes it easier to understand what a module does

There are some .ml files which only contain types, these are instead renamed to .mli and dune's `modules_without_implementation` feature is used.
Executables get an empty .mli (recent versions of dune already do the equivalent of this, but this makes it more obvious).

Drop code from .ml files that now show up as unused.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

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

I'm not going to review this in detail. When this compiles the interfaces are fine.

@edwintorok edwintorok added this pull request to the merge queue May 9, 2025
Merged via the queue into xapi-project:master with commit 1cfaab9 May 9, 2025
17 checks passed
Comment on lines 41 to -43
(* Write out a string *)
let string (output : Xmlm.output) (key : string) (x : string) =
pair output key x
Copy link
Collaborator

Choose a reason for hiding this comment

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

You want to drop this comment too

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.

4 participants