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

docs: update on zoe.installBundle and zoe.installBundleId #1265

Merged
merged 3 commits into from
Jan 22, 2025

Conversation

Chris-Hibbert
Copy link
Collaborator

Update the descriptions of zoe.installBundle() and zoe.installBundleId()

@Chris-Hibbert Chris-Hibbert self-assigned this Jan 21, 2025
Copy link

cloudflare-workers-and-pages bot commented Jan 21, 2025

Deploying documentation with  Cloudflare Pages  Cloudflare Pages

Latest commit: 64c9c90
Status: ✅  Deploy successful!
Preview URL: https://39b9881b.documentation-7tp.pages.dev
Branch Preview URL: https://10872-installbundleid.documentation-7tp.pages.dev

View logs

Copy link

github-actions bot commented Jan 21, 2025

Cloudflare deployment logs are available here

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

I have some suggestions; none critical.

Comment on lines 150 to 153
Takes bundled source code for a Zoe contract as an argument and
installs the code with Zoe. The _bundleLabel_ will be accessible on the
**Installation** object. Returns a **Promise** for an **Installation**
object.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

re "accessible on the Installation object" might as well say how. something like:

accessible on the Installation using E(anInstallation).getBundleLabel().

and / or make Installation into a link.

Copy link
Member

Choose a reason for hiding this comment

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

Should .install(...) be deprecated except for testing?

Discussion such as Agoric/agoric-sdk#3269 (comment) makes it clear that we aim to keep bundles out of vat transcripts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good ideas, all.

Comment on lines 169 to 170
Takes a bundleId for a Zoe contract (often generated in a [coreEval
proposal](/guides/coreeval/local-testnet.html#deploying-contracts-using-core-eval-proposals)
Copy link
Member

Choose a reason for hiding this comment

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

coreEval happens on chain. Bundle generation happens off-chain in a builder script.

I suggest linking to bundling a contract instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@Chris-Hibbert Chris-Hibbert merged commit 60fbdcd into main Jan 22, 2025
6 checks passed
@Chris-Hibbert Chris-Hibbert deleted the 10872-installBundleId branch January 22, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants