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

orch ref docs fixes (WIP) #9768

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft

orch ref docs fixes (WIP) #9768

wants to merge 14 commits into from

Conversation

dckc
Copy link
Member

@dckc dckc commented Jul 24, 2024

goal of this DRAFT is:

closes: #9757

Description / Documentation Considerations

see commit headlines

Security / Scaling / Upgrade Considerations

n / a

Testing Considerations

manual review of @agoric/orchestration reference docs

Copy link

cloudflare-workers-and-pages bot commented Jul 24, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: cb66f49
Status: ✅  Deploy successful!
Preview URL: https://95ef2e48.agoric-sdk.pages.dev
Branch Preview URL: https://dc-orch-ref-docs.agoric-sdk.pages.dev

View logs

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.38%. Comparing base (a14b527) to head (a8b6295).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #9768   +/-   ##
=======================================
  Coverage   56.38%   56.38%           
=======================================
  Files         667      667           
  Lines      207560   207575   +15     
  Branches      340      341    +1     
=======================================
+ Hits       117024   117037   +13     
- Misses      90417    90420    +3     
+ Partials      119      118    -1     
Files Coverage Δ
packages/orchestration/src/chain-info.js 100.00% <100.00%> (ø)
...rchestration/src/exos/cosmos-interchain-service.js 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@@ -23,6 +23,10 @@ import {

const { Vow$ } = NetworkShape; // TODO #9611
/**
* This is the greatest thing since sliced bread.
*
* TODO: private
Copy link
Member

Choose a reason for hiding this comment

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

https://typedoc.org/tags/private/ says it shouldn't be used. I think we want @internal

Copy link
Member Author

Choose a reason for hiding this comment

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

I've struggled to get @internal to work. @mitdralla suggested @private, so I tried that. Also didn't work.

Copy link
Member

Choose a reason for hiding this comment

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

For now I think we should assume we can get it working. Even when it's not omitted it has an INTERNAL tag which is better than nothing. #9808 (comment)

@@ -3,11 +3,21 @@ import { VowShape } from '@agoric/vow';
import { M } from '@endo/patterns';

/**
* @import {TypedPattern} from '@agoric/internal';
* @import {TypedPattern as TPInternal} from '@agoric/internal';
Copy link
Member

Choose a reason for hiding this comment

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

why rename this? If TypedPattern is to be internal (which I support) it should simply have the @internal tag. For that matter, I suppose everything in @agoric/internal should.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem was: The docs showed DelegationShape: TypedPattern<Delegation> but TypedPattern wasn't linked, because it was from the internal package.

So this is an alias in the orchestration so that it gets linked. For example, see:
https://dc-orch-ref-docs.agoric-sdk.pages.dev/variables/_agoric_orchestration.DelegationShape

Moving TypedPattern to @endo/patterns might be a better way to fix this, but I'm not sure how to test that quickly.

Copy link
Member

Choose a reason for hiding this comment

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

can we leave it unlinked? if Delegation is linked, that seems enough to me

Copy link
Member Author

Choose a reason for hiding this comment

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

Delegation is also unlinked... it's from @agoric/cosmic-proto... I wonder if it's straightforward to address that.

Copy link
Member Author

Choose a reason for hiding this comment

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

can we leave it (TypedPattern) unlinked?

I'm making a separate issue about it; we can decide to close it without action:

amessbee and others added 3 commits August 5, 2024 19:58
Added/Fixed a couple of lines in JSDocs comments for `prepareChainHubAdmin` with Dan's help.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incomplete jsdocs for Orchestration API types, variables, methods and parameters
3 participants