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

Rename wasm_instance_new()’s “traps” argument to “trap”. #2478

Merged
merged 1 commit into from
Jul 18, 2021

Conversation

FGasper
Copy link
Contributor

@FGasper FGasper commented Jul 17, 2021

Issue #2472

Description

This clarifies a bit how to properly use the error-handling mechanism in wasm_instance_new(), which only produces 0 or 1 traps, not 2+ as the name “traps” implies.

Review

  • Add a short description of the change to the CHANGELOG.md file

Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

Great catch!

@syrusakbary
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jul 17, 2021
2478: Rename wasm_instance_new()’s “traps” argument to “trap”. r=syrusakbary a=FGasper

Issue #2472

# Description

This clarifies a bit how to properly use the error-handling mechanism in `wasm_instance_new()`, which only produces 0 or 1 traps, not 2+ as the name “traps” implies.

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Felipe Gasper <felipe@felipegasper.com>
@bors
Copy link
Contributor

bors bot commented Jul 17, 2021

Canceled.

@syrusakbary
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jul 17, 2021
2478: Rename wasm_instance_new()’s “traps” argument to “trap”. r=syrusakbary a=FGasper

Issue #2472

# Description

This clarifies a bit how to properly use the error-handling mechanism in `wasm_instance_new()`, which only produces 0 or 1 traps, not 2+ as the name “traps” implies.

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Felipe Gasper <felipe@felipegasper.com>
@bors
Copy link
Contributor

bors bot commented Jul 17, 2021

Build failed:

@syrusakbary
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jul 17, 2021
2478: Rename wasm_instance_new()’s “traps” argument to “trap”. r=syrusakbary a=FGasper

Issue #2472

# Description

This clarifies a bit how to properly use the error-handling mechanism in `wasm_instance_new()`, which only produces 0 or 1 traps, not 2+ as the name “traps” implies.

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Felipe Gasper <felipe@felipegasper.com>
@bors
Copy link
Contributor

bors bot commented Jul 17, 2021

Build failed:

@FGasper
Copy link
Contributor Author

FGasper commented Jul 18, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 18, 2021

🔒 Permission denied

Existing reviewers: click here to make FGasper a reviewer

@syrusakbary
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 18, 2021

@bors bors bot merged commit b137a37 into wasmerio:master Jul 18, 2021
@@ -9,6 +9,8 @@ Looking for changes that affect our C API? See the [C API Changelog](lib/c-api/C
## **[Unreleased]**

### Changed
- [#2478](https://github.com/wasmerio/wasmer/pull/2478) Rename `traps` input to `wasm_instance_new()` to `trap`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect, it should have been written in the lib/c-api/CHANGELOG.md file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in #2481.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry! I noticed that only after committing. Thank you for fixing it.

@Hywan Hywan added 🎉 enhancement New feature! 📚 documentation Do you like to read? 📦 lib-c-api About wasmer-c-api labels Jul 19, 2021
@Hywan Hywan added this to 🌱 In progress in Wasmer Runtime Issue Board via automation Jul 19, 2021
@Hywan Hywan mentioned this pull request Jul 19, 2021
1 task
@Hywan Hywan moved this from 🌱 In progress to 🎉 Done in Wasmer Runtime Issue Board Jul 21, 2021
@syrusakbary syrusakbary added this to the v2.1 milestone Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation Do you like to read? 🎉 enhancement New feature! 📦 lib-c-api About wasmer-c-api
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants