Read build info from package.json #58
Conversation
There was a problem hiding this comment.
I like where this is going but I think this increment is a little more confusing on the way to something better.
If we want the package.json to control the build config, then let's go all the way.
This one would be:
"agoricSyntheticChain": {
"mode": "fromAg0",
},Then appending to the default (main) would be:
"agoricSyntheticChain": {
"mode": "append",
},And a specific branch would be:
"agoricSyntheticChain": {
"mode": "append",
"fromTag": "my-tag",
},Then the CLI command could be simply synthetic-chain build for all cases.
packages/synthetic-chain/cli.ts
Outdated
| @@ -49,16 +52,19 @@ const buildImages = () => { | |||
|
|
|||
| switch (cmd) { | |||
| case 'amend': // alias for backcompat | |||
| case 'append': | |||
| const fromTag = positionals[1] || baseTag; | |||
| case 'append': { // alias for backcompat | |||
There was a problem hiding this comment.
this one is the primary, not the alias. maybe this got moved accidentally from case: 'build':?
There was a problem hiding this comment.
I wanted to deprecate the append command line as well, and keep only build
There was a problem hiding this comment.
Ah gotcha. Let's just drop this case and make the breaking change right now now to have just build
packages/synthetic-chain/cli.ts
Outdated
| @@ -20,7 +21,9 @@ const { positionals, values } = parseArgs({ | |||
| allowPositionals: true, | |||
| }); | |||
|
|
|||
| const allProposals = readProposals(path.resolve('.')); | |||
| const proposalsParent = path.resolve('.'); | |||
There was a problem hiding this comment.
this isn't inaccurate but I think more importantly it's the root of the testing project.
| const proposalsParent = path.resolve('.'); | |
| const root = path.resolve('.'); |
It already is, but admittedly the "from ag0" case is not obviously spelled out, with
I think the "appending" mode should also be the default. At the end of the day we only expect a single a3p image, but multiple integrations in dapps and agoric-sdk. |
mhofman
force-pushed
the
mhofman/build-info-package-json
branch
from
9dfb37b
to
a8976ba
Compare
|
I clarified that using |
packages/synthetic-chain/cli.ts
Outdated
| @@ -49,16 +52,19 @@ const buildImages = () => { | |||
|
|
|||
| switch (cmd) { | |||
| case 'amend': // alias for backcompat | |||
| case 'append': | |||
| const fromTag = positionals[1] || baseTag; | |||
| case 'append': { // alias for backcompat | |||
There was a problem hiding this comment.
Ah gotcha. Let's just drop this case and make the breaking change right now now to have just build
| @@ -17,6 +17,7 @@ sed -i.bak "s/^enable-unsafe-cors =.*/enable-unsafe-cors = true/" "$HOME/.agoric | |||
| sed -i.bak "s/127.0.0.1:26657/0.0.0.0:26657/" "$HOME/.agoric/config/config.toml" | |||
| sed -i.bak "s/cors_allowed_origins = \[\]/cors_allowed_origins = \[\"*\"\]/" "$HOME/.agoric/config/config.toml" | |||
| sed -i.bak '/^\[api]/,/^\[/{s/^enable[[:space:]]*=.*/enable = true/}' "$HOME/.agoric/config/app.toml" | |||
| sed -i.bak "s/^pruning =.*/pruning = \"nothing\"/" "$HOME/.agoric/config/app.toml" | |||
There was a problem hiding this comment.
this should be its own commit but I'll approve to get this in faster.
|
Failure was Since it has to run again, I moved the prune change into its own commit and added the version release commit. |
turadg
force-pushed
the
mhofman/build-info-package-json
branch
from
a8976ba
to
e7855fb
Compare
Oh my bad I didn't mean to include that in this PR, it was a local experiment. But I guess it's there now |
Follow-up to #52, but instead of relying on new commands and positional parameters, the base build info is read from the
package.jsonenclosing theproposals. This allows using a single and simplesynthetic-chain buildcommand, whether in a3p itself or in other repos integrations. It also provides a place in the future to configure the build of other a3p like images.I have manually locally tested this change with Agoric/agoric-sdk#8743.
A new version of the
synthetic-chainpackage still needs to be published if approved.