Skip to content

Conversation

@vogler
Copy link
Collaborator

@vogler vogler commented Jul 12, 2021

I thought we could drop the ocaml-monadic dependency since OCaml introduced definable binding operators in 4.08.0.
However, not having something like ;%bind makes the code a bit ugly to read.
I only changed pml_arinc.ml which compiles - pml_osek.ml is the same.

We can close this if there's no better solution.

@sim642 sim642 added the cleanup Refactoring, clean-up label Jul 13, 2021
@michael-schwarz michael-schwarz marked this pull request as ready for review July 18, 2022 09:41
@michael-schwarz
Copy link
Member

I merged master into this now such that we can decide to merge or close. I personally don't have strong feelings either way, but I think it is a good idea to reduce the number of old open PRs and issues.

@sim642
Copy link
Member

sim642 commented Jul 18, 2022

I only changed pml_arinc.ml which compiles - pml_osek.ml is the same.

Ah, I guess after #735 that doesn't need to be done.

I would be for doing this change since the slight extra ugliness is just in one obscure module and let* () = isn't much more code than ;bind, but it is more standard now, especially since we use let* in some other places as well.
The ocaml-monadic dependency could now be dropped from dune-project, etc entirely though.

@sim642
Copy link
Member

sim642 commented Jul 18, 2022

Actually, the ;bind/let* () could maybe be replaced with an appropriate infix operator definition for >> (i.e. a version of monadic bind that ignores the first value). Although Pml uses some other weird definition of >>, which is completely unrelated, so it's probably easiest to skip this and stay with the changes already made in this PR.

@sim642 sim642 added the setup Dependencies, CI, releasing label Jul 18, 2022
@michael-schwarz michael-schwarz merged commit b441d53 into master Jul 18, 2022
@michael-schwarz michael-schwarz deleted the ocaml-monadic branch July 18, 2022 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Refactoring, clean-up setup Dependencies, CI, releasing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants