Warn unsupported multiple containers of the same type#96
Conversation
anisaoshafi
requested review from
carole-lavillonniere and
silv-io
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new helper function detects when multiple emulators of the same container type are configured and emits a runtime warning. The check is invoked during the container startup flow after configuration retrieval and before container preparation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/container/start.go`:
- Around line 42-44: The warning text emitted when
hasDuplicateContainerTypes(cfg.Containers) is true is misleading; update the
message passed to output.EmitWarning(sink, ...) to refer to "duplicate emulator
definitions in configuration" (or similar) rather than implying multiple
emulators will be run; locate the call to output.EmitWarning in start.go (the
block using hasDuplicateContainerTypes and cfg.Containers) and replace the
message string to clearly state that duplicates were found in the config.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4a758131-c446-4474-b4c3-ef35ce327e5e
📒 Files selected for processing (1)
internal/container/start.go
internal/container/start.go
Outdated
| } | ||
|
|
||
| if hasDuplicateContainerTypes(cfg.Containers) { | ||
| output.EmitWarning(sink, "multiple emulators of the same type are defined in your config; this setup is not supported yet") |
There was a problem hiding this comment.
nit: I guess we didn't establish a wording style guide yet, but I think other messages start capitalized. non blocking but something to think about :D
There was a problem hiding this comment.
Good point, changed it. I'll remember that for next time 🙌🏼
2 participants
Show a warning to users that have multiple containers of the same type defined in their config to avoid confusion when they start lstk:
