deprecate playground#2074
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR narrows provider enumeration to BYOC providers in the shared accessor, updates CLI help/completion and wizard selection to use that subset, and adjusts selector tests to match the reduced provider list. ChangesBYOC provider subset
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.12.2)level=warning msg="The linter 'gomodguard' is deprecated (since v2.12.0) due to: new major version. Replaced by gomodguard_v2." 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.
🧹 Nitpick comments (2)
src/pkg/cli/client/provider_id.go (1)
30-31: Avoid index-coupled filtering and returning a shared slice.
allProviders[2:]depends on ordering and exposes the backing array. Returning a filtered copy is safer and clearer.Proposed refactor
func ByocProviders() []ProviderID { - return allProviders[2:] // skip "auto" and "defang" + providers := make([]ProviderID, 0, len(allProviders)) + for _, p := range allProviders { + if p == ProviderAuto || p == ProviderDefang { + continue + } + providers = append(providers, p) + } + return providers }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pkg/cli/client/provider_id.go` around lines 30 - 31, ByocProviders currently returns allProviders[2:], which is index-dependent and exposes the backing array; change ByocProviders to build and return a new slice by iterating over allProviders and appending only providers whose IDs/names are not "auto" or "defang" (or otherwise match the intended excluded values) so the result is a copied, filtered list; use the ByocProviders function and the allProviders variable to locate the code and ensure the returned slice is newly allocated (not a sub-slice) and not dependent on ordering.src/pkg/stacks/selector_test.go (1)
186-187: Optional: deduplicate repeated provider option literals in tests.The same
providerOptionsslice is repeated across multiple tests; consider a shared test helper/fixture to reduce drift.Also applies to: 262-263, 421-423, 458-460
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pkg/stacks/selector_test.go` around lines 186 - 187, Tests repeatedly declare the same providerOptions slice and pass it into mockEC.On(..., "provider", providerOptions); extract that literal into a shared test fixture (e.g., a package-level var ProviderOptions []string or a helper function ProviderOptions()) and update all occurrences that declare providerOptions and call mockEC.On to use the shared symbol instead; ensure the helper/var is placed in the same test package (selector_test) so existing tests (the places calling mockEC.On with "provider") can import it without changing signatures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pkg/cli/client/provider_id.go`:
- Around line 30-31: ByocProviders currently returns allProviders[2:], which is
index-dependent and exposes the backing array; change ByocProviders to build and
return a new slice by iterating over allProviders and appending only providers
whose IDs/names are not "auto" or "defang" (or otherwise match the intended
excluded values) so the result is a copied, filtered list; use the ByocProviders
function and the allProviders variable to locate the code and ensure the
returned slice is newly allocated (not a sub-slice) and not dependent on
ordering.
In `@src/pkg/stacks/selector_test.go`:
- Around line 186-187: Tests repeatedly declare the same providerOptions slice
and pass it into mockEC.On(..., "provider", providerOptions); extract that
literal into a shared test fixture (e.g., a package-level var ProviderOptions
[]string or a helper function ProviderOptions()) and update all occurrences that
declare providerOptions and call mockEC.On to use the shared symbol instead;
ensure the helper/var is placed in the same test package (selector_test) so
existing tests (the places calling mockEC.On with "provider") can import it
without changing signatures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9fc520c-008b-4e40-98fd-9fc638cafe35
📒 Files selected for processing (4)
src/cmd/cli/command/commands.gosrc/pkg/cli/client/provider_id.gosrc/pkg/stacks/selector_test.gosrc/pkg/stacks/wizard.go
2 participants
Description
remove playground from interactive provider selection and from provider flag options
Linked Issues
Checklist
Summary by CodeRabbit
Release Notes
--provider/-Pflag help text and shell completion now only suggest BYOC provider values.