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

types for deployment scripts #7895

Merged
merged 4 commits into from
Oct 23, 2024
Merged

types for deployment scripts #7895

merged 4 commits into from
Oct 23, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Jun 7, 2023

incidental (I was cleaning out old PRs and thought this was worth getting into master.)

Description

This defines a DeployScriptFunction type and marks many deploy script functions with it. The value of this is someone can command-click through the code to see how it's supposed to work and what the expected endowments are.

To get that working I also had to fix some of the types for bundling, which is another value of landing this.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

We might want to document these scripts better but this doesn't create any new work.

Testing Considerations

Should not affect runtime

@dckc
Copy link
Member

dckc commented Jul 25, 2023

re documentation considerations:

Discussions don't seem to automatically back-link the way issues and PRs do.

@turadg turadg force-pushed the ta/deployment-types branch from 0d4bab8 to 53a6ed3 Compare October 23, 2024 19:35
Copy link

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

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6864614
Status: ✅  Deploy successful!
Preview URL: https://9d6c9ba6.agoric-sdk.pages.dev
Branch Preview URL: https://ta-deployment-types.agoric-sdk.pages.dev

View logs

@turadg turadg requested review from dckc and michaelfig October 23, 2024 19:38
@turadg turadg marked this pull request as ready for review October 23, 2024 19:39
@turadg turadg requested a review from a team as a code owner October 23, 2024 19:39
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

nice. several times I have referred people to these types, only to realize they're on a not-yet-merged-yet branch

* namesByAddress: ERef<NameHub>,
* scratch: ERef<import('@agoric/internal/src/scratch.js').ScratchPad>,
* zoe: ERef<ZoeService>,
* }} CanonicalHome
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a note that home only works in ag-solo mode?

* publishBundle: PublishBundleRef,
* pathResolve: (...path: string[]) => string,
* cacheDir: string,
* }} DeployScriptEndownments
Copy link
Member

Choose a reason for hiding this comment

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

very nice.

I don't see this in/near https://agoric-sdk.pages.dev/modules/_agoric_deploy_script_support . I guess that's not essential to the IDE use case, though.

@turadg turadg force-pushed the ta/deployment-types branch from 53a6ed3 to 6864614 Compare October 23, 2024 20:50
@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Oct 23, 2024
@mergify mergify bot merged commit bcab39d into master Oct 23, 2024
90 checks passed
@mergify mergify bot deleted the ta/deployment-types branch October 23, 2024 21:25
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
Development

Successfully merging this pull request may close these issues.

2 participants