Skip to content

Conversation

@Julow
Copy link
Contributor

@Julow Julow commented Mar 5, 2025

This PR removes the use of lwt_ppx by rewriting let%lwt, match%lwt, etc.. to let*, Lwt.bind and other direct calls to Lwt functions.
Templates are also updated.

This was done using this tool: https://github.com/Julow/lwt_ppx_to_let_syntax/

The first commit runs OCamlformat to remove parasitic diffs in the other commits as the rewrite tool relies on OCamlformat too.

This is a first step toward moving from Lwt to effect-based concurrency. Here are related PRs on the other ocsigen projects: ocsigen/eliom#815 ocsigen/ocsigen-toolkit#236 ocsigen/ocsipersist#6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure how to update this file so I let the tool run on it. Any guideline on how to setup the right environment to run the update command ?

@Julow Julow changed the title Remove lwt ppx Remove lwt_ppx dependency Mar 5, 2025
src/os_db.ml Outdated
( (fun password -> Bcrypt.string_of_hash (Bcrypt.hash password))
, fun _ password1 password2 ->
( (fun password -> Bcrypt.string_of_hash (Bcrypt.hash password)),
fun _ password1 password2 ->
Copy link
Member

Choose a reason for hiding this comment

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

Did you ocamlformat with our .ocamlformat?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understood that this file is generated from os_db.ppx.ml ? Generating it again will solve this.

This other file is not fully formatted either but I get large diffs messing up the readability of the queries so I tried to keep as few diffs as possible.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe. In that case we should probably remove it from the repository, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not generated automatically during build. Anyone knows how to update os_db.ml ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

os_db.ml is uptodate ! This is ready to be merged.

This file is re-generated by running 'make rebuild-os-db' in an
environment created with the template's 'make db-init db-create db-schema'.
@balat balat merged commit 638f875 into ocsigen:master Apr 2, 2025
5 of 7 checks passed
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