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

Add more parameters to Func #1078

Merged
merged 3 commits into from
Dec 18, 2019
Merged

Add more parameters to Func #1078

merged 3 commits into from
Dec 18, 2019

Conversation

Jasper-Bekkers
Copy link
Contributor

@Jasper-Bekkers Jasper-Bekkers commented Dec 18, 2019 •
edited
Loading

Description

We ran into the issue that we needed to pass more then the available number of parameters to wasm, so I've extended the parameter count for Func to have max 26 parameters now.

Review

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

@Jasper-Bekkers Jasper-Bekkers mentioned this pull request Dec 18, 2019
Open
26 tasks
Hywan
Hywan approved these changes Dec 18, 2019
View reviewed changes
Copy link
Contributor

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

LGTM!

@Hywan
Copy link
Contributor

Hywan commented Dec 18, 2019

I was wondering when someone would need more than 12 parameters. Here we are ;-).

@Hywan Hywan self-assigned this Dec 18, 2019
@Jasper-Bekkers
Copy link
Contributor Author

I was wondering when someone would need more than 12 parameters. Here we are ;-).

I'm not exactly happy about having to need them, but that's life.

The linter seems to complain about formatting, I'd personally prefer the single-line macro invocations but I can understand the project not wanting exceptions for it so I'll leave that up to you guys to decide :-)

@Hywan
Copy link
Contributor

Hywan commented Dec 18, 2019 •
edited
Loading

The linter is rustfmt. Please follow its advice. We don't have any particular opinions on this topic, we let the linter decides for us ;-).

@Hywan
Copy link
Contributor

Hywan commented Dec 18, 2019

Or, add #[rustfmt::skip], that's fine too! It's up to you :-).

@Hywan Hywan added 🎉 enhancement New feature! 📦 lib-deprecated About the deprecated crates labels Dec 18, 2019
@Hywan
Copy link
Contributor

Hywan commented Dec 18, 2019

bors r+

bors bot added a commit that referenced this pull request Dec 18, 2019
@bors @Jasper-Bekkers
Merge #1078
fd17651 
1078: Add more parameters to Func r=Hywan a=Jasper-Bekkers

<!-- 
Prior to submitting a PR, review the CONTRIBUTING.md document for recommendations on how to test:
https://github.com/wasmerio/wasmer/blob/master/CONTRIBUTING.md#pull-requests

-->

# Description
We ran into the issue that we needed to pass more then the available number of parameters to wasm, so I've extended the parameter count for `Func` to have max 26 parameters now.

# Review

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


Co-authored-by: Jasper-Bekkers <bekkers@gmail.com>
@Jasper-Bekkers
Copy link
Contributor Author

I'll add the #[rustfmt::skip] I think it's visually a bit nicer and more inline with the formatting for the rest of the file.

@bors
Copy link
Contributor

bors bot commented Dec 18, 2019

Build failed

@Hywan
Copy link
Contributor

Hywan commented Dec 18, 2019

bors r+

bors bot added a commit that referenced this pull request Dec 18, 2019
@bors @Jasper-Bekkers
Merge #1078
6229346 
1078: Add more parameters to Func r=Hywan a=Jasper-Bekkers

<!-- 
Prior to submitting a PR, review the CONTRIBUTING.md document for recommendations on how to test:
https://github.com/wasmerio/wasmer/blob/master/CONTRIBUTING.md#pull-requests

-->

# Description
We ran into the issue that we needed to pass more then the available number of parameters to wasm, so I've extended the parameter count for `Func` to have max 26 parameters now.

# Review

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


Co-authored-by: Jasper-Bekkers <bekkers@gmail.com>
@Jasper-Bekkers
Copy link
Contributor Author

It looks like TravisCI is failing because of some network issue

@Hywan
Copy link
Contributor

Hywan commented Dec 18, 2019

Yup, I agree. Let's see in a moment.

@Hywan
Copy link
Contributor

Hywan commented Dec 18, 2019

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 18, 2019

Already running a review

@bors
Copy link
Contributor

bors bot commented Dec 18, 2019

Build succeeded

@bors bors bot merged commit 22b3a06 into wasmerio:master Dec 18, 2019
@Jasper-Bekkers Jasper-Bekkers deleted the add-func-parameters branch December 18, 2019 13:34
@Hywan
Copy link
Contributor

Hywan commented Dec 18, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hywan Hywan Hywan approved these changes

@bjfish bjfish Awaiting requested review from bjfish

Assignees

@Hywan Hywan

Labels
🎉 enhancement New feature! 📦 lib-deprecated About the deprecated crates
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Jasper-Bekkers @Hywan