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

fix: simplify autoswap helper APIs #1732

Merged
merged 1 commit into from
Sep 14, 2020
Merged

Conversation

dtribble
Copy link
Member

Switch the calling convention for autoswap helpers to
passing positional arguments. The use of a params object
doesn't work very well in JS. It's exacerbated if we
need to use harden, and the value is reduced now that
we have pervasive types on helpers.

Given that, please consider this approach.

Copy link
Contributor

@katelynsills katelynsills left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but let's keep the assertDefined

@dtribble dtribble force-pushed the autoswap-positional-helpers branch 2 times, most recently from b4943ac to 8333a4e Compare September 10, 2020 22:15
Copy link
Contributor

@Chris-Hibbert Chris-Hibbert left a comment

Choose a reason for hiding this comment

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

I like this change. Great idea!

@katelynsills
Copy link
Contributor

@dtribble Can we merge this? I can help get it merged by doing the rebasing if that would be helpful.

@katelynsills katelynsills added the Zoe package: Zoe label Sep 14, 2020
@katelynsills katelynsills self-assigned this Sep 14, 2020
@dtribble
Copy link
Member Author

Apologies. I thought that the merge had happened. Yes please take it over.

Switch the calling convention for autoswap helpers to
passing positional arguments. The use of a params object
doesn't work very well in JS. It's exacerbated if we
need to use `harden`, and the value is reduced now that
we have pervasive types on helpers.
@katelynsills katelynsills force-pushed the autoswap-positional-helpers branch from 8333a4e to 749bc64 Compare September 14, 2020 22:26
@katelynsills katelynsills merged commit 068f4b1 into master Sep 14, 2020
@katelynsills katelynsills deleted the autoswap-positional-helpers branch September 14, 2020 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Zoe package: Zoe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants