Skip to content

Conversation

@jordanstephens
Copy link
Member

@jordanstephens jordanstephens commented Dec 9, 2025

Description

In #1687, I moved the existing deployments list from behind a ? keypress into the spotlight.

In the first deployment I did without the agent, I saw this:

GCP_PROJECT_ID=jordan-project-463223 defang -Pgcp deploy
 * Using Google Cloud Platform provider from command line flag
 ! This project has already deployed elsewhere:

 - AWS account "381492210770" in region %us-west-2

New output looks like this:

GCP_PROJECT_ID=jordan-project-463223 defang -Pgcp deploy
 * Using Google Cloud Platform provider from command line flag
This project has already deployed elsewhere:
 - AWS account "381492210770" in us-west-2

This PR does the following:

  • remove that % before the region
  • remove the extra newline
  • remove the word "region" from each line
  • avoid printing duplicate lines
  • avoid printing "This project has already deployed elsewhere" as a warning, so it doesn't get repeated at the bottom when the command finishes.

Linked Issues

Checklist

  • I have performed a self-review of my code
  • I have added appropriate tests
  • I have updated the Defang CLI docs and/or README to reflect my changes, if necessary

Summary by CodeRabbit

  • Bug Fixes

    • Corrected region display formatting in account information output.
  • Chores

    • Streamlined deployment confirmation: existing deployment locations are now shown in a clearer, sorted and deduplicated list before confirmation.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 9, 2025

Walkthrough

Centralized deployment-location presentation by adding printExistingDeployments(), called from confirmDeploymentToNewLocation; removed the prior NonInteractive-specific warning path. Also fixed AccountInfo.String() region concatenation to append the region value directly.

Changes

Cohort / File(s) Change Summary
Deployment display refactoring
src/cmd/cli/command/compose.go
Added printExistingDeployments(existingDeployments []*defangv1.Deployment) to print sorted, deduplicated deployment locations; updated confirmDeploymentToNewLocation() to call the new helper and removed the previous NonInteractive-specific warning messaging.
String formatting fix
src/pkg/cli/client/account_info.go
Corrected AccountInfo.String() region construction from using a literal placeholder to directly appending the region (" in " + a.Region).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Review sorting and deduplication logic in printExistingDeployments for correctness and edge cases (nil slices, identical entries).
  • Verify confirmDeploymentToNewLocation behavior in interactive and non-interactive modes after removal of the prior warning path.
  • Quick check of AccountInfo.String() output formatting across typical inputs.

Poem

🐇 A tidy function hops into view,
Sorting and pruning the deployment queue,
Regions now whisper the right little name,
Deduped and aligned — neat code, not a shame,
I nibble a carrot and celebrate anew. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Clean up existing deployment printing' directly and accurately summarizes the main change: refactoring how existing deployments are printed, including fixing formatting issues and centralizing the logic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jordan/fix-printing-existing-deployments

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0313f7c and 12983b4.

📒 Files selected for processing (1)
  • src/cmd/cli/command/compose.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/cmd/cli/command/compose.go
⏰ 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). (2)
  • GitHub Check: go-test
  • GitHub Check: Analyze (go)

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"
level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/cmd/cli/command/compose.go (1)

232-244: printExistingDeployments: centralized, sorted, de‑duplicated output looks good

The helper cleanly centralizes existing-deployment display, and sorting + slices.Compact ensures stable, non-duplicated lines without extra blank output. The use of cliClient.AccountInfo here also leverages the fixed String() representation so there’s no stray % or hard-coded “region” wording anymore.

If you want a tiny optimization, you could preallocate the slice:

deploymentStrings := make([]string, 0, len(existingDeployments))

but that’s strictly optional.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 996c4d1 and 0313f7c.

📒 Files selected for processing (2)
  • src/cmd/cli/command/compose.go (1 hunks)
  • src/pkg/cli/client/account_info.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cmd/cli/command/compose.go (4)
src/protos/io/defang/v1/fabric.pb.go (7)
  • Deployment (3035-3050)
  • Deployment (3063-3063)
  • Deployment (3078-3080)
  • Provider (28-28)
  • Provider (66-68)
  • Provider (70-72)
  • Provider (79-81)
src/pkg/term/colorizer.go (1)
  • Info (302-304)
src/pkg/cli/client/provider_id.go (1)
  • ProviderID (10-10)
src/pkg/cli/client/account_info.go (1)
  • AccountInfo (5-10)
⏰ 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 (2)
src/pkg/cli/client/account_info.go (1)

12-17: AccountInfo.String(): formatting fix looks correct

Building the region suffix with simple concatenation now avoids leaking the literal %s and drops the extra “region” word, so strings like "<provider> account <id> in <region>" render as intended. No further changes needed here.

src/cmd/cli/command/compose.go (1)

246-258: [rewritten review comment]
[classification tag]

@jordanstephens jordanstephens merged commit 0c267bd into main Dec 10, 2025
14 checks passed
@jordanstephens jordanstephens deleted the jordan/fix-printing-existing-deployments branch December 10, 2025 18:52
@coderabbitai coderabbitai bot mentioned this pull request Dec 21, 2025
3 tasks
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.

3 participants