Skip to content

Conversation

@endersonmaia
Copy link
Contributor

@endersonmaia endersonmaia commented Feb 4, 2025

This pull request includes several changes to the @cartesi/cli package, focusing on centralizing the definition of Docker images and refactoring the Docker Compose files to use a base configuration. The most important changes are outlined below:

Refactoring and Centralization:

  • Created a single place to define the cartesi/sdk and cartesi/rollups-node images. (.changeset/selfish-dots-perform.md)

Code Updates:

  • Imported DEFAULT_SDK from config.js and used it in the Run command to set the CARTESI_SDK_IMAGE environment variable. (apps/cli/src/commands/run.ts) [1] [2]

Docker Compose Refactoring:

  • Updated docker-compose-anvil.yaml to extend from docker-compose-base.yaml for sdk_image and rollups_node_image services. (apps/cli/src/node/docker-compose-anvil.yaml) [1] [2]
  • Added docker-compose-base.yaml with definitions for sdk_image and rollups_node_image services. (apps/cli/src/node/docker-compose-base.yaml)
  • Refactored other Docker Compose files (docker-compose-bundler.yaml, docker-compose-database.yaml, docker-compose-espresso.yaml, docker-compose-paymaster.yaml, docker-compose-validator.yaml) to extend from docker-compose-base.yaml for consistency. [1] [2] [3] [4] [5]

@endersonmaia endersonmaia requested a review from tuler February 4, 2025 13:21
@changeset-bot
Copy link

changeset-bot bot commented Feb 4, 2025

🦋 Changeset detected

Latest commit: 855012a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cartesi/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2025

Coverage Report for ./apps/cli

Status Category Percentage Covered / Total
🔵 Lines 30.97% 293 / 946
🔵 Statements 31.1% 298 / 958
🔵 Functions 36.95% 51 / 138
🔵 Branches 27.88% 121 / 434
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
apps/cli/src/commands/run.ts 0% 0% 0% 0% 10-235
Generated in workflow #267 for commit 855012a by the Vitest Coverage Report Action

@endersonmaia endersonmaia force-pushed the feature/single-place-image-definition branch from a27b565 to f350102 Compare February 4, 2025 13:25
@endersonmaia endersonmaia self-assigned this Feb 4, 2025
@endersonmaia endersonmaia force-pushed the feature/single-place-image-definition branch from f350102 to 2fc76ba Compare February 4, 2025 15:24
@endersonmaia endersonmaia force-pushed the feature/single-place-image-definition branch from 2fc76ba to a6a9af4 Compare February 21, 2025 13:37
Copy link
Member

@tuler tuler left a comment

Choose a reason for hiding this comment

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

This use of extends is quite unorthodox. I don't think I like it.

@endersonmaia
Copy link
Contributor Author

This use of extends is quite unorthodox. I don't think I like it.

It's a new feature, and it makes our code more maintainable, IMO.

When you use cartesi run ... --dry-run to look at the the YAML output, it's a common output without extends, for ex.

@endersonmaia endersonmaia force-pushed the feature/cli-run-v2 branch 2 times, most recently from a82afdd to a7e55c5 Compare February 27, 2025 13:18
@tuler tuler force-pushed the feature/cli-run-v2 branch 5 times, most recently from 39558cf to d496299 Compare March 6, 2025 18:31
@tuler tuler force-pushed the feature/cli-run-v2 branch 2 times, most recently from b295744 to 48ddef2 Compare March 14, 2025 20:31
@tuler tuler mentioned this pull request Mar 15, 2025
@tuler tuler force-pushed the feature/cli-run-v2 branch 2 times, most recently from caf7a2f to c2a395d Compare March 15, 2025 16:05
@endersonmaia
Copy link
Contributor Author

Solved at #116 using environment variable replacement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants