Skip to content

Conversation

@surma
Copy link
Contributor

@surma surma commented Mar 6, 2024

I am currently in need of setting features from the C API, so I figured I’d take a stab at fixing the TODO.

Not sure if this is the way you’d wanna go about it, though. Happy to adjust!

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Hi @surma , thanks, this looks like the right C API change here!

Code lgtm. Please just add a note to CHANGELOG.md and a small test to test/example/c-api-kitchen-sink.c (after adding it, running ./scripts/auto_update_tests.py example will update the output for that file).

@surma
Copy link
Contributor Author

surma commented Mar 6, 2024

@kripken I hope I did this right :)

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Looks good aside from one comment!

@kripken
Copy link
Member

kripken commented Mar 6, 2024

Looks like there is a CI error too, on lint: https://github.com/WebAssembly/binaryen/actions/runs/8177938588/job/22362367130?pr=6380

surma and others added 2 commits March 6, 2024 22:44
Co-authored-by: Alon Zakai <alonzakai@gmail.com>
@surma
Copy link
Contributor Author

surma commented Mar 6, 2024

Fixed!

@surma surma force-pushed the surma/c-api-features branch from 3e1af54 to 43487fa Compare March 7, 2024 07:39
@kripken kripken enabled auto-merge (squash) March 7, 2024 17:02
@kripken kripken merged commit ab9e3f7 into WebAssembly:main Mar 7, 2024
@gkdn gkdn mentioned this pull request Aug 31, 2024
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