-
Notifications
You must be signed in to change notification settings - Fork 206
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
generate HTML documentation from TS/JSDoc #8343
Conversation
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.
great start!
of course, it makes lots of problems easier to see...
@@ -341,6 +341,7 @@ type ChainBootstrapSpaceT = { | |||
* Vault Factory. ONLY FOR DISASTER RECOVERY | |||
*/ | |||
instancePrivateArgs: Map<Instance, unknown>; | |||
mints: MintsVat; |
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.
mints
is a demo/test-only thing. It prints money for free.
Is it straightforward to silence the other end of whatever prompted this?
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.
not the other end, but I'll make this optional since it's not always defined
* vatAdminSvc: VatAdminSve; | ||
* vatAdminSvc: VatAdminSvc; |
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.
👏
tsconfig.build.json
Outdated
@@ -0,0 +1,36 @@ | |||
/* Visit https://aka.ms/tsconfig.json to read more about this file */ |
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.
shortened URL makes me uneasy; was this generated? if not, how about...
/* Visit https://aka.ms/tsconfig.json to read more about this file */ | |
/* Visit https://www.typescriptlang.org/tsconfig to read more about this file */ |
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 think it's the default config that TS makes. I'm fine changing it
* @param {import('@agoric/vat-data').Baggage} baggage | ||
* @param {import('@agoric/swingset-liveslots').Baggage} baggage |
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 don't see Baggage
in the generated docs for swingset-liveslots. I wonder what's up here.
packages/vat-data/src/index.js
Outdated
/** @typedef {import('@agoric/swingset-liveslots').DurableKindHandle} DurableKindHandle */ | ||
/** @template T @typedef {import('@agoric/swingset-liveslots').DefineKindOptions<T>} DefineKindOptions */ | ||
/** typedef {import('@agoric/swingset-liveslots').Baggage} Baggage */ |
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.
removing the @
from @typedef
was on purpose? Another problem with aliasing Baggage
?
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.
oh, I need to come back to this. Probably explain why it's there.
packages/vats/src/vat-board.js
Outdated
* @param {import('@agoric/vat-data').Baggage} baggage | ||
* @param {import('@agoric/swingset-liveslots').Baggage} baggage |
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 wondered whether exposing so many contracts etc. to swingset-liveslots
was a problem. I looked at the depgraph from scripts/graph.sh
. It shows vat-data
is right next door, so it doesn't seem like a problem.
refs: #4291
Description
Use TypeDoc to generate API docs from the JSDoc and TypeScript annotations
Demo: https://agoric-api-docs-demo.pages.dev/
Security Considerations
Perhaps more visibility of internal APIs, but not showing anything that's not already in the repo.
Scaling Considerations
n/a
Documentation Considerations
Should not be published for external consumption in its current state. We'll need to use tags like @beta or @internal and config options to exclude those from public builds.
I figure we'll want multiple targets:
Testing Considerations
Let's get this working and in our own CI to ensure it continues to work. We can kick the tires and figure out requirements before publish visibility.
Upgrade Considerations
n/a