Conversation
WalkthroughThis PR extends the Deployment message in protobuf to include a deployment mode field, then propagates this change through the CLI by initializing the mode when creating deployments and displaying it in the deployment listings table. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter" Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/pkg/cli/deploymentsList_test.go (1)
83-84: Consider adding test cases for other deployment modes.The test currently only validates
MODE_UNSPECIFIED. While this covers the default case, adding test cases forDEVELOPMENT,STAGING, andPRODUCTIONmodes would provide better coverage of the new functionality.src/pkg/cli/deploymentsList.go (1)
55-55: Consider more user-friendly mode display format.Using
.String()displays the full enum name (e.g., "MODE_UNSPECIFIED", "DEVELOPMENT"). Consider transforming these for better readability:
- "MODE_UNSPECIFIED" → "-" or "Unspecified"
- "DEVELOPMENT" → "Development" (title case)
- "STAGING" → "Staging"
- "PRODUCTION" → "Production"
Example transformation:
- Mode: d.Mode.String(), + Mode: formatMode(d.Mode),Then add a helper function:
func formatMode(mode defangv1.DeploymentMode) string { if mode == defangv1.DeploymentMode_MODE_UNSPECIFIED { return "-" } name := mode.String() return strings.Title(strings.ToLower(name)) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/protos/io/defang/v1/fabric.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (4)
src/pkg/cli/composeUp.go(1 hunks)src/pkg/cli/deploymentsList.go(3 hunks)src/pkg/cli/deploymentsList_test.go(2 hunks)src/protos/io/defang/v1/fabric.proto(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/pkg/cli/deploymentsList_test.go (3)
src/pkg/modes/modes.go (1)
Mode(12-12)src/protos/io/defang/v1/fabric.pb.go (1)
DeploymentMode_MODE_UNSPECIFIED(86-86)src/pkg/crun/local/local.go (1)
Local(21-27)
src/pkg/cli/deploymentsList.go (2)
src/pkg/modes/modes.go (1)
Mode(12-12)src/pkg/term/table.go (1)
Table(11-13)
src/pkg/cli/composeUp.go (1)
src/pkg/modes/modes.go (1)
Mode(12-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
src/protos/io/defang/v1/fabric.proto (1)
455-455: LGTM! Proto field addition is backward compatible.The new
modefield is properly defined with a sequential field number and uses the existingDeploymentModeenum. Since proto3 treats all fields as optional by default, this change is backward compatible with existing clients and servers.src/pkg/cli/composeUp.go (1)
164-164: LGTM! Mode is properly propagated to deployment metadata.The deployment mode from the parameters is correctly set in the
PutDeploymentRequest, ensuring the mode is persisted with each deployment.src/pkg/cli/deploymentsList_test.go (1)
41-41: LGTM! Test properly includes the new Mode field.The mock deployment correctly includes the
Modefield set toMODE_UNSPECIFIED, which is the appropriate default value for deployments without an explicitly set mode.src/pkg/cli/deploymentsList.go (2)
21-21: LGTM! Mode field properly added to output structure.The
Modefield is correctly added to thePrintDeploymentstruct for display purposes.
69-69: Note: Column order change may affect output parsers.Inserting the "Mode" column between "Deployment" and "DeployedAt" changes the table layout. If any scripts or tools parse this output programmatically, they may break. Consider whether adding the Mode column at the end would be less disruptive, or if this breaking change is acceptable for your use case.
Description
Save deployment mode for each deployment and print it when listing deployments
Linked Issues
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.