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

8868 gen submissions #9009

Merged
merged 5 commits into from
Feb 29, 2024
Merged

8868 gen submissions #9009

merged 5 commits into from
Feb 29, 2024

Conversation

Chris-Hibbert
Copy link
Contributor

refs: a3p#87
refs: 8868

Description

Rather than requiring every new core-eval proposal to edit generate-a3p-submission.sh, each proposal can add details to its package.json, and all required submissions will be generated from that.

Security Considerations

None: internal tooling only

Scaling Considerations

None: internal tooling only

Documentation Considerations

Added comments in some of the READMEs

Testing Considerations

Verified it worked on #8911

Upgrade Considerations

This is support for testing upgrades. It doesn't involve any code on chain.

@Chris-Hibbert Chris-Hibbert added the tooling repo-wide infrastructure label Feb 28, 2024
@Chris-Hibbert Chris-Hibbert self-assigned this Feb 28, 2024
Base automatically changed from a3p-test-zcfBundleCap to master February 28, 2024 19:15
@Chris-Hibbert
Copy link
Contributor Author

@turadg, It's confused about the commits. I'll rebase and fix it.

@@ -23,7 +23,8 @@
]
},
"scripts": {
"agops": "yarn --cwd /usr/src/agoric-sdk/ --silent agops"
"agops": "yarn --cwd /usr/src/agoric-sdk/ --silent agops",
"build:submission": "../../../scripts/generate-a3p-submission.sh probe-zcf-bundle a:upgrade-next probeZcfBundle probe-submission"
Copy link
Member

Choose a reason for hiding this comment

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

the proposal package must not take a dependency on SDK scripts.

What you could do instead is put these params into agoricProposal and have the top-level script read those to know what to build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's another try. It required extra escaping because of directory names containing dashes.

@@ -6,6 +6,8 @@
"upgradeInfo": {
"coreProposals": []
},
"submission": "probe-zcf-bundle",
"probe-zcf-bundle": "a:upgrade-next probeZcfBundle probe-submission",
Copy link
Member

Choose a reason for hiding this comment

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

a:upgrade-next is implied by the proposal package.

Copy link
Member

Choose a reason for hiding this comment

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

Let's also be clear that this isn't part of the synthetic-chain API.

  "sdk-generate": [
    "probe-zcf-bundle probeZcfBundle probe-submission"
  ]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -6,7 +6,7 @@
"scripts": {
"build": "yarn run build:sdk && yarn run build:submission && yarn run build:synthetic-chain",
"build:sdk": "make -C ../packages/deployment docker-build-sdk",
"build:submission": "../scripts/generate-a3p-submission.sh",
"build:submission": "../scripts/generate-a3p-submission-dirs.sh",
Copy link
Member

Choose a reason for hiding this comment

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

nit: plural

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Comment on lines 5 to 6
"submission": "test-localchain",
"test-localchain": "b:localchain"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"submission": "test-localchain",
"test-localchain": "b:localchain"
"sdk-generate": [
"test-localchain"
]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cone

Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

Looks good!

@Chris-Hibbert Chris-Hibbert added the automerge:squash Automatically squash merge label Feb 29, 2024
@mergify mergify bot merged commit f33b371 into master Feb 29, 2024
66 checks passed
@mergify mergify bot deleted the 8868-gen-submissions branch February 29, 2024 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge tooling repo-wide infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants