-
Notifications
You must be signed in to change notification settings - Fork 39
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): Remove makeLocalAccount #1208
base: main
Are you sure you want to change the base?
Conversation
Deploying documentation with Cloudflare Pages
|
Cloudflare deployment logs are available here |
@@ -16,14 +16,6 @@ Retrieves the chain information and provides access to chain-specific methods. S | |||
const chain = await orchestrator.getChain('chainName') | |||
``` | |||
|
|||
### makeLocalAccount |
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.
This is accurate but I suspect not complete. If this repo is about a guide to using the API, shouldn't we also indicate how to make a local account? (getChain('agoric')
If this is just a presentation of the API, why is it defined manually here? We can generate API docs from the API repo.
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.
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.
this is a work in progress
Sure, but part of my question is about doing less work here. I'm concerned we'll spend time redescribing the API in this repo when we can just import the API output from agoric-sdk.
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 digress but have a look at devrel's current plan for orchestration docs(slack link). Look forward to feedback to improve this, but we believe that a certain amount of redundancy will enhance DX. Also, I will drag you to #1201 for feedback 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.
Agree Chain
object documentation is missing from this page, but this is an improvement since makeLocalAccount
guides the developer down the wrong path.
yarn format
should make the Lint Markdown job pass
I no longer see @ivanlei - as the author i'll let you close this |
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
Per Agoric/agoric-sdk#10106 the method
orchestrator.makeLocalAccount
will be removed.This PR removes docs about that method. There are no other instances of this method in the docs repo.