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(bindings/d): add D bindings support #5181

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

kassane
Copy link
Contributor

@kassane kassane commented Oct 14, 2024

Requirements

How to work D Binding?

importC read opendal.h on <root_dir>/bindings/c/include to auto-generate a binding.
Note: the pragma attribute(push,nogc,nothrow) is new feature (not released yet).
https://dlang.org/changelog/pending.html#dmd.importc-pragma-stc

Note

Currently, cbindgen PR hasn't been merged to support D. So I've ruled out using the fork. But that doesn't stop me from using the current solution.

cc: @Xuanwo

@kassane kassane requested review from Xuanwo and PsiACE as code owners October 14, 2024 20:49
@kassane kassane force-pushed the d-binding branch 2 times, most recently from 761ff8c to f98f0e3 Compare October 15, 2024 12:24
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.

Hi @kassane, thank you so much for your efforts on this! I've never written in D before, so I'm unable to offer detailed advice. However, I've left some suggestions regarding the public API; please have a look.

.github/workflows/ci_bindings_d.yml Show resolved Hide resolved
.github/workflows/ci_bindings_d.yml Show resolved Hide resolved
bindings/d/dub.json Outdated Show resolved Hide resolved
bindings/d/source/opendal/operator.d Outdated Show resolved Hide resolved
bindings/d/source/opendal/operator.d Outdated Show resolved Hide resolved
bindings/d/source/opendal/operator.d Outdated Show resolved Hide resolved
@kassane kassane force-pushed the d-binding branch 2 times, most recently from 80bd29b to 2c418ef Compare October 16, 2024 17:15
@kassane kassane requested a review from Xuanwo October 16, 2024 17:22
@kassane

This comment was marked as resolved.

@kassane kassane changed the title D bindings support feat(bindings/swift): add D bindings support Oct 17, 2024
@kassane kassane changed the title feat(bindings/swift): add D bindings support feat(bindings/d): add D bindings support Oct 17, 2024
@kassane kassane force-pushed the d-binding branch 2 times, most recently from 1a9b85d to 7ddea57 Compare October 21, 2024 14:23
@kassane
Copy link
Contributor Author

kassane commented Oct 21, 2024

Rebased, single commit!

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, thank you @kassane for working on this.

bindings/d/source/opendal/operator.d Outdated Show resolved Hide resolved
* `std.paralellism` support added (write/read/list)
* CI/CD test
* rename `is_exists` - ref.: apache#5199

Signed-off-by: Matheus C. França  <[email protected]>
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.

Thank you @kassane for working on this! I believe it's a good start point. Would you like to share this with Dlang community to collect some feedback?

@Xuanwo Xuanwo merged commit 5e074cc into apache:main Oct 22, 2024
30 checks passed
@kassane kassane deleted the d-binding branch October 22, 2024 13:14
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.

2 participants