-
Notifications
You must be signed in to change notification settings - Fork 217
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
feat(far): new package @agoric/far
#4144
Conversation
9161429
to
ac44615
Compare
Hmmm... In #3833 the suggested name was far. It's shorter. Perhaps we could even lose the @agoric/ qualifier in due course... This is clearly a user-visible feature, not just a chore. |
1516aff
to
41036a4
Compare
@agoric/far
41036a4
to
8be558c
Compare
@agoric/far
@agoric/far
👍
For branding and logical grouping of packages, I'd like to see this pushed down into
Thanks, adjusted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exports seem to differ between index.js and the README
yes, quite likely. In fact, I considered consolidating the eventual-send.md and far.md sections. Keeping them separate seems ok for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good stuff.
I'm curious how much of the new package boilerplate you had to do from memory. I don't see anything about it in https://github.com/Agoric/agoric-sdk/blob/master/CONTRIBUTING.md
|
||
/** | ||
* @template T | ||
* @typedef {import('@agoric/eventual-send').EOnly<T>} EOnly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EOnly<T>
is sort of new to me; I'm used to ERef<T>
. hm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added comments, and good catch: this package should export ERef<T>
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, too late to beat the automerge. I'll do a follow-on.
closes: #3833
Description
Create a new package
@agoric/far
, as described in theREADME.md
, to cover the common cases of clients of the Agoric distributed objects system.After this toehold, a follow-on PR can convert many of our redundant:
to just:
The eventual goal is to reduce the dependencies required to implement this package.
Security Considerations
Documentation Considerations
I'd like to get @dckc's involvement to see if it helps simplify example code in https://github.com/Agoric/documentation.
Testing Considerations
Tests were lifted from
@agoric/eventual-send
and@agoric/marshal
, and pruned to test only the features exported by this package.