9757 Orchestration docs timebox #10050
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
220d4b2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c8cf169f.agoric-sdk.pages.dev |
| Branch Preview URL: | https://9757-timebox.agoric-sdk.pages.dev |
| @@ -32,7 +32,7 @@ | |||
| "prettier-plugin-jsdoc": "^1.3.0", | |||
| "prettier-plugin-sh": "^0.14.0", | |||
| "type-coverage": "^2.27.1", | |||
| "typedoc": "^0.26.4", | |||
| "typedoc": "^0.26.7", | |||
There was a problem hiding this comment.
are there any particular features that we're using from this release? or was this just housekeeping?
I notice it's less than a day old. For ref: c56c478
There was a problem hiding this comment.
maybe hoping for better visibility support?
| * const contract = (zcf, privateArgs, zone, tools) => { ... }; | ||
| * export const start = withOrchestration(contract); | ||
| * ``` | ||
| * |
There was a problem hiding this comment.
I don't see withOrchestration on https://9757-timebox.agoric-sdk.pages.dev/modules/_agoric_orchestration :-/
There was a problem hiding this comment.
oh yeah, that got stalled: #9808
I'll try to solve that here
| /** | ||
| * Transactions for doing staking operations on an individual account. | ||
| * | ||
| * @see {@link https://docs.cosmos.network/main/build/modules/staking#messages x/staking messages} |
There was a problem hiding this comment.
a comma or something between the links would help.
it's a little hard to tell that there are 2 of them.
| * Cosmos-specific methods to extend `OrchestrationAccountI`, parameterized | ||
| * by `CosmosChainInfo`. | ||
| * | ||
| * In particular, if the chain info includes a staking token, `StakingAccountActions` |
| @@ -234,6 +234,9 @@ export interface OrchestrationAccountI { | |||
| getPublicTopics: () => Promise<Record<string, ResolvedPublicTopic<unknown>>>; | |||
| } | |||
|
|
|||
| /** | |||
| * An async-flow suitable for use in contracts using Orchestration | |||
There was a problem hiding this comment.
"async-flow" is a pretty opaque term here.
I suggest moving (most of) the "Orchestration flows" section of the README to here.
| @@ -246,6 +246,7 @@ export interface OrchestrationFlow<CT = unknown> { | |||
| * The type must be able to express transfers across different chains and transports. | |||
| * | |||
| * NOTE Expected to change, so consider an opaque structure. | |||
| * @internal | |||
There was a problem hiding this comment.
looks like you got this working. nice!
| /** | ||
| * agoricNames key for ChainInfo hub | ||
| * | ||
| * @internal |
There was a problem hiding this comment.
These agoricNames keys don't seem internal to me. They show up all the way to clients via vstorage.
There was a problem hiding this comment.
good point, I'll revert those. I still think they should go into an enum, but that can wait
| @@ -42,7 +41,9 @@ export const IcaAccountI = M.interface('IcaAccount', { | |||
| reactivate: M.call().returns(VowShape), | |||
| }); | |||
|
|
|||
| // XXX none of these modifiers are working to exclude this type from api-docs | |||
There was a problem hiding this comment.
in some cases, no. maybe on types that are transitively included
There was a problem hiding this comment.
in some cases, no. maybe on types that are transitively included
| export type IBCConnectionInfo = { | ||
| export interface IBCConnectionInfo { |
There was a problem hiding this comment.
I'd expect to use interfaces for things with methods, but this is an ordinary data structure. Why not leave it as a type alias?
There was a problem hiding this comment.
I guess being able to refer to each property with a link is nice.
There was a problem hiding this comment.
I read some perf docs recently: https://github.com/microsoft/TypeScript/wiki/Performance#preferring-interfaces-over-intersections
| export type CosmosAssetInfo = { | ||
| export interface CosmosAssetInfo extends Record<string, unknown> { |
There was a problem hiding this comment.
likewise, seems fine as a type alias
| /** @import {NameAdmin} from '@agoric/vats'; */ | ||
|
|
||
| /** @typedef {CosmosChainInfo | EthChainInfo} ChainInfo */ | ||
| /** | ||
| * Info used to build a {@link Chain} object - naming, connections, etc. |
There was a problem hiding this comment.
nit: naming stuck out to me as something to improve. maybe:
| * Info used to build a {@link Chain} object - naming, connections, etc. | |
| * Info used to build a {@link Chain} object - channel, connection, and denom info. |
| @@ -78,12 +78,14 @@ const knownChains = /** @satisfies {Record<string, ChainInfo>} */ ( | |||
| * @internal | |||
| */ | |||
|
|
|||
| // TODO(#9572): include this in registerChain | |||
| @@ -133,6 +134,7 @@ export const DenomAmountShape = { denom: DenomShape, value: M.bigint() }; | |||
| /** @type {TypedPattern<AmountArg>} */ | |||
| export const AmountArgShape = M.or(AmountShape, DenomAmountShape); | |||
|
|
|||
| /** @type {TypedPattern<RequestQuery>} */ | |||
There was a problem hiding this comment.
RequestQuery['data'] is Uint8Array. Was going to suggest using TypedPattern<JsonSafe<RequestQuery>>, but we need to preserve RequestQuery['height']: bigint
refs: #9757
Description
A bunch of docs improvements to get into the u17 release.
This doesn't do all the things specified in the ticket so leaves open:
Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
We'll need tutorial docs. I think some of the
@examplesuggestions make more sense there.Testing Considerations
CI
Upgrade Considerations
not yet deployed