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

feat: Add OCaml Binding #2757

Merged
merged 21 commits into from
Aug 4, 2023
Merged

feat: Add OCaml Binding #2757

merged 21 commits into from
Aug 4, 2023

Conversation

Ranxy
Copy link
Contributor

@Ranxy Ranxy commented Aug 1, 2023

ref #2756

Added a simple OCaml binding to implement the blocking operator.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 1, 2023

Woooooooooooow

@Ranxy
Copy link
Contributor Author

Ranxy commented Aug 1, 2023

It seems that ocaml/setup-ocaml@v2 cannot be used directly in CI

@oowl
Copy link
Member

oowl commented Aug 1, 2023

It seems that ocaml/setup-ocaml@v2 cannot be used directly in CI

Maybe it's related to #2693 (comment), I think you can directly use shell command to install ocaml enviroment.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 2, 2023

As mentioned by @oowl, we require approval from ASF Infra for this action. The action appears to be maintained by ocaml.org, which is a trustworthy source. I have submitted a ticket at https://issues.apache.org/jira/browse/INFRA-24856 and now we need to wait for their response.

@Ranxy
Copy link
Contributor Author

Ranxy commented Aug 2, 2023

In order to package this project with ocaml's package manager opam, it seems that the files bindings/ocaml/dune-project and bindings/ocaml/opendal.opam need to be moved to the top level directory of the project. Will this be a problem?

@Ranxy
Copy link
Contributor Author

Ranxy commented Aug 2, 2023

In order to package this project with ocaml's package manager opam, it seems that the files bindings/ocaml/dune-project and bindings/ocaml/opendal.opam need to be moved to the top level directory of the project. Will this be a problem?

This is because opam will take the directory of dune-project as the root directory of the project, and copy this directory to a separate directory for compilation and packaging. If dune-project is in bindings/ocaml, then it cannot be built after copying this directory alone. Because this is part of the entire workspace in the rust project and cannot be compiled separately.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 2, 2023

Will this be a problem?

Yes, it's better to avoid such things from happening. Does the link core/ work here?

@Ranxy
Copy link
Contributor Author

Ranxy commented Aug 2, 2023

Will this be a problem?

Yes, it's better to avoid such things from happening. Does the link core/ work here?

It is not very clear at the moment, let me try to find an alternative solution to solve this problem.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 2, 2023

It is not very clear at the moment, let me try to find an alternative solution to solve this problem.

Thanks for the research. I don't think there's a block preventing us from merging this PR, so we can continue working on this binding and address any package issues later.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 3, 2023

Hi, setup-ocaml has been approved. Please add some new commits to have a try.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Xuanwo Xuanwo changed the title Ocaml binding feat: Add OCaml Binding Aug 4, 2023
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Mostly LGTM! It's quiet intereting to see opendal on OCaml.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Great, mostly LGTM!

Would you like to start a tracking issue for the future issues of ocaml binding?

@Xuanwo Xuanwo merged commit 0fcb442 into apache:main Aug 4, 2023
20 checks passed
@Ranxy
Copy link
Contributor Author

Ranxy commented Aug 4, 2023

Great, mostly LGTM!

Would you like to start a tracking issue for the future issues of ocaml binding?

Of course it's exciting.

@Ranxy Ranxy mentioned this pull request Aug 4, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants