-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: support for multiple sdks and abstract machine making into create_image script in sdk #24
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.
Install changesets and add a changset file where you propose the minor version bump for the cartesi/sdk package.
apps/cli/src/commands/build.ts
Outdated
@@ -19,6 +19,7 @@ type ImageInfo = { | |||
env: string[]; | |||
ramSize: string; | |||
sdkVersion: string; | |||
sdkType: string; |
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 suggest making this variable consistent with the label.
sdkName
CARTESI_LABEL_SDK_NAME
io.cartesi.rollups.sdk_name
ee546bd
to
bb054ff
Compare
…te_image script in sdk Signed-off-by: Carsten Munk <[email protected]>
bb054ff
to
fa82ab2
Compare
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.
@stskeeps changeset will take care of updating the CHANGELOG and package.json files when changes hit the main branch, a GH Actions job will create/update the Release PR.
Since this PR has changes for two packages with a dependency between them, we'd need to have the changes in separate PRs.
The first one is with changes to the cartesi/sdk package, so we can release it first. Another PR with changes to the cartesi/cli making use of the new cartesi/sdk.
OK, so close this pull request and I've opened as a start #26 - this is the style desired right? |
This probably requires upping CARTESI_DEFAULT_SDK_VERSION as well.