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

implement appending #52

Merged
merged 7 commits into from
Jan 8, 2024
Merged

implement appending #52

merged 7 commits into from
Jan 8, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Jan 7, 2024

Closes: Agoric/agoric-sdk#8605

This adds an append command that executes the proposals on top of an existing image. It takes an optional tag, defaulting to main.

I published an rc0 so this could be used in Agoric/agoric-sdk#8635. Then I realized the CLI names could be improved so I added a commit for that. I didn't fixup the previous because rc0 had been cut. I'll cut a release after this PR merges. I also need to set up a auto-publishing on merge.

@turadg turadg changed the title implement amending implement appending Jan 8, 2024
@turadg turadg requested review from mhofman and dckc January 8, 2024 17:15
@turadg turadg marked this pull request as ready for review January 8, 2024 17:15
Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

I am somewhat out of my depth here to be able to review this.

One thing that confuses me is whether the "rebuild" command is meant to be used outside of this repo? In which case, what does it "rebuild"?

packages/synthetic-chain/cli.ts Outdated Show resolved Hide resolved
packages/synthetic-chain/cli.ts Outdated Show resolved Hide resolved
@turadg
Copy link
Member Author

turadg commented Jan 8, 2024

One thing that confuses me is whether the "rebuild" command is meant to be used outside of this repo? In which case, what does it "rebuild"?

It currently only makes sense to use in this repo. It rebuilds all proposals starting from agzero. Another option is to eliminate this command and always "append" but appending to agZero is a special case. I figured it was better to separate the use case instead of detecting the tag name.

@mhofman
Copy link
Member

mhofman commented Jan 8, 2024

I find "build" less confusing than "rebuild" which implies some existing information exists.

I'm also thinking that "build" command should also take a base image (not just tag)?

@turadg
Copy link
Member Author

turadg commented Jan 8, 2024

I find "build" less confusing than "rebuild" which implies some existing information exists.

I'm open to that. It was build first, but in adding append it's also building. I chose rebuild to mean "build from ag-zero". Any other ideas?

I'm also thinking that "build" command should also take a base image (not just tag)?

You mean the append command? We could take the whole image but it opens room for error since this tool only yet supports images built with agoric-3-proposals as the base.

@mhofman
Copy link
Member

mhofman commented Jan 8, 2024

You mean the append command? We could take the whole image but it opens room for error since this tool only yet supports images built with agoric-3-proposals as the base.

I actually meant build, but it falls in the same category of assuming we're starting from a specific ag0 tag. We can figure that out later.

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.

I'm a little fuzzy on the concept, but it looks like forward progress.

@turadg turadg merged commit 959d961 into main Jan 8, 2024
1 check passed
@turadg turadg deleted the ta/amend branch January 8, 2024 20:02
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.

tool to test proposal on top of agoric-3 chain
3 participants