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

remove orch.makeLocalAccount #10260

Merged
merged 2 commits into from
Oct 16, 2024
Merged

remove orch.makeLocalAccount #10260

merged 2 commits into from
Oct 16, 2024

Conversation

0xpatrickdev
Copy link
Member

@0xpatrickdev 0xpatrickdev commented Oct 11, 2024

closes: #10106

Description

There doesn't seem to be a motivating use case for exposing a maker for a low-level LocalChainAccount in the Orchestrator, so lets remove it to avoid misdirection. Instead, consumers can call orch.getChain('agoric').then(c => c.makeAccount()) to get a LocalOrchestrationAccount.

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

docs.agoric.com already reflects this: Agoric/documentation#1208 (comment)

Testing Considerations

Removing code, so existing tests suffice

Upgrade Considerations

Library code that will go out in a NPM Orch Release

Copy link

cloudflare-workers-and-pages bot commented Oct 11, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: fa13c4e
Status: ✅  Deploy successful!
Preview URL: https://564961cf.agoric-sdk.pages.dev
Branch Preview URL: https://10106-remove-makelocalaccoun.agoric-sdk.pages.dev

View logs

Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

I take the fact that no tests fail without it as evidence it isn't necessary for any use cases of the product

* chainHub: ChainHub;
* localchain: Remote<LocalChain>;
* makeRecorderKit: MakeRecorderKit;
Copy link
Member

Choose a reason for hiding this comment

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

I expect some of these will need to be restored later but I agree it's better to leave them out until they're used

@0xpatrickdev 0xpatrickdev added the automerge:rebase Automatically rebase updates, then merge label Oct 11, 2024
@0xpatrickdev 0xpatrickdev force-pushed the 10106-remove-makeLocalAccount branch 2 times, most recently from b6e0883 to 3356749 Compare October 15, 2024 19:55
@mergify mergify bot merged commit 94aaebc into master Oct 16, 2024
80 checks passed
@mergify mergify bot deleted the 10106-remove-makeLocalAccount branch October 16, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
2 participants