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.

Sign up for GitHub

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

Jump to bottom

refactor: resolve c pointers const #5171

Merged
merged 7 commits into from
Oct 14, 2024
Merged

refactor: resolve c pointers const #5171

merged 7 commits into from
Oct 14, 2024

Conversation

tisonkun
Copy link
Member

@tisonkun tisonkun commented Oct 8, 2024

Draft to try.

@tisonkun tisonkun requested review from Xuanwo and PsiACE as code owners October 8, 2024 06:04
@tisonkun tisonkun mentioned this pull request Oct 8, 2024
Merged
tisonkun
tisonkun commented Oct 8, 2024
View reviewed changes
bindings/go/operator.go Outdated Show resolved Hide resolved
yuchanns
yuchanns reviewed Oct 8, 2024
View reviewed changes
bindings/go/operator.go Outdated Show resolved Hide resolved
bindings/go/operator.go Outdated Show resolved Hide resolved
@tisonkun tisonkun force-pushed the c-pointers branch 2 times, most recently from 9ac2373 to 32dba0c Compare October 8, 2024 07:14
tisonkun
tisonkun commented Oct 8, 2024
View reviewed changes
bindings/zig/test/bdd.zig Outdated Show resolved Hide resolved
@tisonkun
Copy link
Member Author

tisonkun commented Oct 8, 2024

@yuchanns now compile can work but test failed. Please see https://github.com/apache/opendal/actions/runs/11230441983/job/31217839150.

tisonkun
tisonkun commented Oct 8, 2024
View reviewed changes
bindings/c/src/types.rs Outdated Show resolved Hide resolved
@tisonkun tisonkun requested a review from yuchanns October 8, 2024 08:39
@tisonkun
Copy link
Member Author

tisonkun commented Oct 8, 2024

@yuchanns I revert changes in Golang because we're now accept *mut for free again. But the other type can change so there is still failure. I don't know how to debug it locally so I'd delegate it to you to figure out.

@tisonkun tisonkun force-pushed the c-pointers branch 2 times, most recently from 0492f88 to 3a1e39f Compare October 8, 2024 09:32
@tisonkun
Copy link
Member Author

tisonkun commented Oct 8, 2024

@yuchanns I fixed the go cases now.

@tisonkun
Copy link
Member Author

tisonkun commented Oct 8, 2024

No. It's still failed =。=

@yuchanns
Copy link
Member

yuchanns commented Oct 8, 2024

so I'd delegate it to you to figure out.

I will be back at Oct. 13th.

@tisonkun
Copy link
Member Author

tisonkun commented Oct 8, 2024

so I'd delegate it to you to figure out.

I will be back at Oct. 13th.

Yeah. No hurry. Enjoy your vacation!

@yuchanns yuchanns mentioned this pull request Oct 14, 2024
Merged
@tisonkun @yuchanns
refactor: resolve c pointers const
a976ace 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun @yuchanns
fix zig cases
069f5b3 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun @yuchanns
fix swift cases
d705197 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun @yuchanns
fix go cases
2be8fa1 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun @yuchanns
pass pointer to opendal_bytes_free
1bde51d 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun @yuchanns
try fix go case
4fd2af9 
Signed-off-by: tison <wander4096@gmail.com>
@yuchanns
Copy link
Member

yuchanns commented Oct 14, 2024 •
edited
Loading

Oops! The rebase overwrites your real commits. Sorry.

Since #5179 has been merged, the go-binding can now be easily debugged. See https://github.com/apache/opendal/tree/main/bindings/go#development

And I will debug the go part in this PR

@yuchanns
fix(bindings/go): make opendal_bytes pointer
109f2af 
Signed-off-by: Hanchin Hsieh <me@yuchanns.xyz>
@yuchanns
Copy link
Member

The Go binding CI is green now. @tisonkun

Xuanwo
Xuanwo approved these changes Oct 14, 2024
View reviewed changes
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 @tisonkun for working on this, and thank you @yuchanns for fixing golang part.

@Xuanwo Xuanwo merged commit 2a332f3 into main Oct 14, 2024
33 checks passed
@Xuanwo Xuanwo deleted the c-pointers branch October 14, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

@PsiACE PsiACE Awaiting requested review from PsiACE PsiACE is a code owner

@yuchanns yuchanns Awaiting requested review from yuchanns

Assignees
No one assigned
Labels
releases-note/refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tisonkun @yuchanns @Xuanwo