Rename packages #1730
Rename packages #1730
Conversation
This ensures we can see debug messages during CLI startup.
WalkthroughThis PR removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/login/login.go (1)
22-24: Critical: Interface signature doesn't match implementation.The
AuthServiceinterface at line 23 declares:login(ctx context.Context, client client.FabricClient, fabric string, flow LoginFlow, mcpClient string)But the
OpenAuthServiceimplementation at line 28 declares:login(ctx context.Context, fabric client.FabricClient, cluster string, flow LoginFlow, mcpClient string)The parameter names differ:
- FabricClient parameter:
client(interface) vsfabric(implementation)- string parameter:
fabric(interface) vscluster(implementation)The interface definition needs to be updated to match the implementation.
🔎 Proposed fix
type AuthService interface { - login(ctx context.Context, client client.FabricClient, fabric string, flow LoginFlow, mcpClient string) (string, error) + login(ctx context.Context, fabric client.FabricClient, cluster string, flow LoginFlow, mcpClient string) (string, error) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
src/cmd/cli/command/cd.go(2 hunks)src/cmd/cli/command/commands.go(21 hunks)src/cmd/cli/command/commands_test.go(5 hunks)src/cmd/cli/command/compose.go(11 hunks)src/cmd/cli/command/compose_test.go(2 hunks)src/cmd/cli/command/estimate.go(2 hunks)src/cmd/cli/command/globals.go(3 hunks)src/cmd/cli/command/globals_test.go(7 hunks)src/cmd/cli/command/mcp.go(2 hunks)src/cmd/cli/command/stack_test.go(4 hunks)src/cmd/cli/command/workspace.go(2 hunks)src/pkg/agent/agent.go(2 hunks)src/pkg/agent/tools/create_aws_stack.go(2 hunks)src/pkg/agent/tools/create_gcp_stack.go(2 hunks)src/pkg/agent/tools/default_tool_cli.go(3 hunks)src/pkg/agent/tools/deploy.go(1 hunks)src/pkg/agent/tools/deploy_test.go(2 hunks)src/pkg/agent/tools/destroy.go(3 hunks)src/pkg/agent/tools/estimate.go(4 hunks)src/pkg/agent/tools/interfaces.go(1 hunks)src/pkg/agent/tools/listConfig.go(3 hunks)src/pkg/agent/tools/logs.go(2 hunks)src/pkg/agent/tools/provider.go(3 hunks)src/pkg/agent/tools/removeConfig.go(3 hunks)src/pkg/agent/tools/services.go(3 hunks)src/pkg/agent/tools/services_test.go(3 hunks)src/pkg/agent/tools/setConfig.go(3 hunks)src/pkg/cli/client/cluster.go(3 hunks)src/pkg/cli/client/cluster_test.go(1 hunks)src/pkg/cli/composeUp.go(1 hunks)src/pkg/cli/configList_test.go(2 hunks)src/pkg/cli/connect.go(1 hunks)src/pkg/cli/destroy_test.go(2 hunks)src/pkg/cli/getServices_test.go(2 hunks)src/pkg/cli/preview.go(1 hunks)src/pkg/cli/tail.go(1 hunks)src/pkg/cli/whoami_test.go(3 hunks)src/pkg/login/login.go(3 hunks)src/pkg/login/login_test.go(3 hunks)src/pkg/mcp/mcp_server.go(3 hunks)src/pkg/mcp/setup_test.go(0 hunks)src/pkg/stacks/stacks_test.go(14 hunks)src/pkg/stacks/wizard.go(5 hunks)src/pkg/stacks/wizard_test.go(26 hunks)src/pkg/track/track.go(1 hunks)
💤 Files with no reviewable changes (1)
- src/pkg/mcp/setup_test.go
🧰 Additional context used
🧬 Code graph analysis (42)
src/pkg/track/track.go (1)
src/pkg/utils.go (1)
GetenvBool(56-59)
src/pkg/cli/composeUp.go (2)
src/pkg/cli/client/client.go (1)
FabricClient(17-44)src/pkg/cli/client/provider.go (1)
Provider(43-69)
src/pkg/agent/tools/deploy.go (2)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)
src/pkg/login/login_test.go (1)
src/pkg/cli/client/cluster.go (2)
GetExistingToken(44-69)GetTokenFile(40-42)
src/cmd/cli/command/stack_test.go (2)
src/pkg/cli/client/provider_id.go (2)
ProviderAWS(15-15)ProviderGCP(17-17)src/pkg/modes/modes.go (2)
Mode(12-12)ModeAffordable(16-16)
src/cmd/cli/command/mcp.go (1)
src/pkg/cli/client/state.go (1)
StateDir(15-15)
src/pkg/cli/destroy_test.go (2)
src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)src/pkg/cli/client/client.go (1)
FabricClient(17-44)
src/pkg/stacks/wizard_test.go (2)
src/pkg/cli/client/provider_id.go (4)
ProviderAWS(15-15)ProviderGCP(17-17)ProviderDefang(14-14)ProviderDO(16-16)src/pkg/stacks/stacks.go (1)
StackParameters(17-24)
src/pkg/agent/agent.go (1)
src/pkg/cli/client/cluster.go (2)
GetExistingToken(44-69)NormalizeHost(18-26)
src/pkg/agent/tools/create_gcp_stack.go (1)
src/pkg/cli/client/provider_id.go (1)
ProviderGCP(17-17)
src/cmd/cli/command/compose_test.go (2)
src/pkg/cli/client/provider_id.go (1)
ProviderDefang(14-14)src/pkg/cli/client/cluster.go (1)
DefaultCluster(14-14)
src/pkg/agent/tools/removeConfig.go (2)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)
src/cmd/cli/command/cd.go (1)
src/pkg/cli/client/projectName.go (1)
LoadProjectNameWithFallback(10-26)
src/pkg/agent/tools/create_aws_stack.go (1)
src/pkg/cli/client/provider_id.go (1)
ProviderAWS(15-15)
src/pkg/agent/tools/logs.go (1)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)
src/pkg/stacks/wizard.go (2)
src/pkg/cli/client/provider_id.go (6)
ProviderAuto(13-13)AllProviders(30-32)ProviderID(10-10)ProviderDefang(14-14)ProviderAWS(15-15)ProviderGCP(17-17)src/pkg/cli/client/region.go (1)
GetRegion(5-16)
src/cmd/cli/command/workspace.go (1)
src/pkg/cli/client/cluster.go (1)
GetExistingToken(44-69)
src/pkg/cli/tail.go (1)
src/pkg/term/colorizer.go (1)
Warn(310-312)
src/pkg/agent/tools/services.go (4)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)src/pkg/mcp/tools/tools.go (1)
StackConfig(14-17)src/pkg/cli/getServices.go (1)
ErrNoServices(17-19)
src/pkg/stacks/stacks_test.go (1)
src/pkg/cli/client/provider_id.go (4)
ProviderID(10-10)ProviderAWS(15-15)ProviderGCP(17-17)ProviderDO(16-16)
src/pkg/agent/tools/destroy.go (2)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)
src/pkg/cli/client/cluster.go (3)
src/pkg/cli/client/state.go (1)
StateDir(15-15)src/pkg/utils.go (1)
Getenv(48-53)src/pkg/term/colorizer.go (1)
Debug(294-296)
src/pkg/cli/preview.go (3)
src/pkg/cli/client/client.go (1)
FabricClient(17-44)src/pkg/cli/client/provider.go (1)
Provider(43-69)src/pkg/cli/composeUp.go (1)
ComposeUpParams(24-29)
src/pkg/mcp/mcp_server.go (1)
src/pkg/cli/client/provider_id.go (1)
ProviderID(10-10)
src/pkg/agent/tools/setConfig.go (2)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)
src/pkg/agent/tools/estimate.go (5)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)src/pkg/cli/connect.go (1)
Connect(16-18)src/pkg/cli/client/provider_id.go (1)
ProviderID(10-10)src/pkg/cli/estimate.go (1)
RunEstimate(23-41)
src/pkg/agent/tools/listConfig.go (5)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)src/pkg/agent/tools/default_tool_cli.go (1)
StackConfig(19-22)src/pkg/mcp/mcp_server.go (1)
StackConfig(46-46)src/pkg/mcp/tools/tools.go (1)
StackConfig(14-17)
src/pkg/cli/getServices_test.go (2)
src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)src/pkg/cli/client/client.go (1)
FabricClient(17-44)
src/pkg/cli/configList_test.go (2)
src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)src/pkg/cli/client/client.go (1)
FabricClient(17-44)
src/pkg/cli/connect.go (1)
src/pkg/cli/client/cluster.go (2)
NormalizeHost(18-26)GetExistingToken(44-69)
src/cmd/cli/command/globals_test.go (3)
src/pkg/stacks/stacks.go (1)
StackParameters(17-24)src/pkg/cli/client/provider_id.go (4)
ProviderAuto(13-13)ProviderAWS(15-15)ProviderGCP(17-17)ProviderDefang(14-14)src/pkg/cli/client/cluster.go (1)
DefangFabric(16-16)
src/cmd/cli/command/commands.go (6)
src/pkg/cli/client/cluster.go (1)
GetExistingToken(44-69)src/pkg/cli/client/pretty_error.go (1)
PrettyError(12-23)src/pkg/cli/client/provider.go (2)
Provider(43-69)Loader(71-74)src/pkg/cli/client/provider_id.go (7)
AllProviders(30-32)ProviderID(10-10)ProviderDefang(14-14)ProviderAWS(15-15)ProviderDO(16-16)ProviderGCP(17-17)ProviderAuto(13-13)src/pkg/cli/client/projectName.go (1)
LoadProjectNameWithFallback(10-26)src/pkg/cli/compose/loader.go (1)
Loader(38-41)
src/pkg/agent/tools/deploy_test.go (3)
src/pkg/login/login.go (1)
InteractiveLoginMCP(47-49)src/pkg/cli/client/grpc.go (1)
GrpcClient(19-26)src/pkg/cli/client/caniuse.go (1)
CanIUseProvider(10-43)
src/cmd/cli/command/compose.go (7)
src/pkg/cli/client/provider.go (1)
Provider(43-69)src/pkg/cli/client/provider_id.go (2)
ProviderDefang(14-14)ProviderID(10-10)src/pkg/cli/client/cluster.go (1)
DefaultCluster(14-14)src/pkg/cli/client/account_info.go (1)
AccountInfo(5-10)src/pkg/cli/client/pretty_error.go (1)
PrettyError(12-23)src/pkg/cli/client/errors.go (1)
ErrDeploymentFailed(5-8)src/pkg/cli/client/projectName.go (1)
LoadProjectNameWithFallback(10-26)
src/cmd/cli/command/estimate.go (3)
src/pkg/cli/client/provider_id.go (4)
ProviderAuto(13-13)ProviderID(10-10)ProviderAWS(15-15)ProviderGCP(17-17)src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)src/pkg/cli/client/region.go (1)
GetRegion(5-16)
src/pkg/cli/whoami_test.go (5)
src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)src/pkg/cli/whoami.go (2)
Whoami(25-59)ShowAccountData(14-23)src/protos/io/defang/v1/fabric.pb.go (4)
Provider(28-28)Provider(66-68)Provider(70-72)Provider(79-81)src/pkg/cli/client/provider.go (1)
Provider(43-69)src/pkg/cli/client/provider_id.go (1)
ProviderDefang(14-14)
src/pkg/agent/tools/services_test.go (2)
src/pkg/cli/client/grpc.go (1)
GrpcClient(19-26)src/pkg/cli/composeUp.go (2)
ComposeUp(32-178)ComposeUpParams(24-29)
src/pkg/agent/tools/default_tool_cli.go (5)
src/pkg/cli/client/grpc.go (1)
GrpcClient(19-26)src/pkg/cli/client/provider_id.go (1)
ProviderID(10-10)src/pkg/cli/connect.go (1)
NewProvider(61-75)src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)src/pkg/cli/client/client.go (1)
FabricClient(17-44)
src/pkg/agent/tools/interfaces.go (7)
src/pkg/cli/client/caniuse.go (1)
CanIUseProvider(10-43)src/pkg/cli/client/grpc.go (1)
GrpcClient(19-26)src/pkg/cli/connect.go (1)
NewProvider(61-75)src/pkg/cli/getServices.go (1)
GetServices(28-42)src/pkg/cli/client/projectName.go (1)
LoadProjectNameWithFallback(10-26)src/pkg/cli/client/provider_id.go (1)
ProviderID(10-10)src/pkg/modes/modes.go (1)
Mode(12-12)
src/cmd/cli/command/commands_test.go (5)
src/pkg/clouds/aws/secrets.go (2)
SsmParametersAPI(14-20)SsmClientOverride(22-22)src/pkg/cli/client/grpc.go (1)
GrpcClient(19-26)src/pkg/cli/client/mock.go (1)
MockLoader(162-165)src/pkg/cli/compose/loader.go (1)
Project(25-25)src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)
src/pkg/agent/tools/provider.go (4)
src/pkg/cli/connect.go (1)
NewProvider(61-75)src/pkg/cli/client/provider_id.go (1)
ProviderID(10-10)src/pkg/cli/client/client.go (1)
FabricClient(17-44)src/pkg/stacks/stacks.go (1)
StackParameters(17-24)
src/pkg/login/login.go (3)
src/pkg/cli/client/client.go (1)
FabricClient(17-44)src/pkg/cli/client/cluster.go (3)
SaveAccessToken(77-85)NormalizeHost(18-26)GetWebIdentityTokenFile(71-75)src/pkg/login/agree_tos.go (1)
NonInteractiveAgreeToS(46-57)
⏰ 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: Analyze (go)
- GitHub Check: go-test
🔇 Additional comments (61)
src/pkg/agent/tools/services_test.go (1)
69-69: LGTM: Parameter renaming improves semantic clarity.The consistent renaming of the
clientparameter tofabricacross all five MockCLI methods accurately reflects that these*client.GrpcClientinstances connect to a fabric controller. The naming is now uniform and more descriptive.Also applies to: 73-73, 85-85, 93-93, 109-109
src/pkg/cli/tail.go (1)
142-142: LGTM!The TODO comment correctly references
client.PrettyErrorto align with the updated import path, consistent with the package rename refactor.src/pkg/cli/connect.go (1)
23-24: LGTM!The calls to
client.NormalizeHostandclient.GetExistingTokencorrectly reference the consolidated client package, replacing the previous cluster package usage. The implementations are confirmed to exist insrc/pkg/cli/client/cluster.go.src/pkg/cli/whoami_test.go (1)
10-10: LGTM!The test correctly updates to the unaliased
clientimport and usesclient.PlaygroundProviderandclient.ProviderDefangconsistently with the package rename refactor.Also applies to: 42-42, 54-54, 60-60
src/pkg/agent/tools/create_gcp_stack.go (1)
8-8: LGTM!The import and
client.ProviderGCPreference are correctly updated to align with the package rename.Also applies to: 36-36
src/pkg/agent/tools/deploy_test.go (1)
59-62: LGTM!Renaming the parameter from
clienttofabriccorrectly avoids shadowing theclientpackage import, improving code clarity. This aligns with the naming used in the actual implementations (e.g.,login.gousesfabric client.FabricClient).Also applies to: 80-83
src/pkg/track/track.go (1)
8-8: LGTM! Clean import alias migration.The import alias has been successfully removed and the type alias has been updated to reflect the direct
clientpackage import. This change aligns with the PR's objective to standardize on direct imports.Also applies to: 16-16
src/pkg/cli/client/cluster.go (2)
1-1: LGTM! Package merge executed correctly.The package declaration has been updated from
clustertoclient, successfully merging the cluster functionality into the cli/client package as described in the PR objectives.
28-41: LGTM! Consistent parameter renaming throughout.The parameter renaming from
fabrictoclusterhas been applied consistently across all functions (tokenStorageName,GetTokenFile,GetExistingToken,GetWebIdentityTokenFile,SaveAccessToken). This improves semantic clarity and aligns with the package's purpose.Also applies to: 44-80
src/pkg/cli/client/cluster_test.go (1)
1-1: LGTM! Test package updated to match production code.The test package declaration correctly reflects the package merge from
clustertoclient.src/pkg/agent/tools/default_tool_cli.go (2)
10-10: LGTM! Import alias successfully removed.The import has been updated from the aliased
cliClientto the directclientpackage, consistent with the PR-wide migration.
30-116: LGTM! All type references consistently updated.All function signatures have been systematically updated to use the direct
clientpackage types (client.GrpcClient,client.Provider,client.ProviderID,client.Loader,client.FabricClient,client.PlaygroundProvider). The migration is complete and consistent throughout the file.src/cmd/cli/command/mcp.go (1)
9-9: LGTM! Import and reference updated consistently.The import alias has been removed and the
StateDirreference has been correctly updated to use the directclientpackage.Also applies to: 40-40
src/cmd/cli/command/cd.go (1)
8-8: LGTM! Import and function call updated correctly.The import alias has been removed and the
LoadProjectNameWithFallbackfunction call has been updated to use the directclientpackage reference.Also applies to: 45-45
src/pkg/login/login_test.go (1)
22-22: LGTM! Test updated to reflect package merge.All function calls have been correctly updated from
cluster.GetExistingTokenandcluster.GetTokenFiletoclient.GetExistingTokenandclient.GetTokenFile, reflecting the merge of the cluster package into the client package. Test logic remains unchanged.Also applies to: 30-30, 37-37, 66-66, 125-125
src/pkg/cli/configList_test.go (1)
9-9: LGTM! Test updated with direct client package usage.The import alias has been removed and the
PlaygroundProvidertype reference has been correctly updated to use the directclientpackage.Also applies to: 54-54
src/cmd/cli/command/stack_test.go (1)
7-7: LGTM! Import alias removal completed correctly.The import path has been updated from an alias to direct package reference, and all provider constant usages are updated consistently throughout the test file.
Also applies to: 45-45, 51-51, 92-92, 102-102
src/pkg/agent/tools/create_aws_stack.go (1)
8-8: LGTM! Package refactoring applied correctly.Import path updated to direct client package reference and provider constant usage updated accordingly.
Also applies to: 36-36
src/cmd/cli/command/workspace.go (1)
10-10: LGTM! Cluster package merge reflected correctly.The token retrieval logic now uses
client.GetExistingTokeninstead ofcluster.GetExistingToken, reflecting the merge of the cluster package into cli/client. Functionality remains unchanged.Also applies to: 19-19
src/cmd/cli/command/compose_test.go (1)
8-8: LGTM! Test updated for package refactoring.Import alias removed and both provider and cluster constants now reference the unified client package. Test behavior remains unchanged.
Also applies to: 33-34
src/pkg/agent/agent.go (1)
17-17: LGTM! Agent updated for cluster package merge.Token retrieval and host normalization now use the client package, reflecting the merge of cluster functionality into cli/client. Agent behavior remains unchanged.
Also applies to: 37-37, 40-40
src/pkg/stacks/wizard_test.go (1)
9-9: LGTM! Comprehensive test suite updated consistently.Import alias removed and all provider constant references updated throughout the extensive test cases. Test coverage and behavior remain unchanged.
Also applies to: 126-126, 152-152, 180-180, 206-206, 228-228, 248-248, 483-483, 492-492, 504-504, 513-513, 525-525, 534-534, 549-549, 558-558, 573-573, 592-592, 610-610, 618-618, 630-630, 639-639, 649-649, 658-658, 668-668, 677-677, 685-685, 704-704, 724-724
src/cmd/cli/command/globals_test.go (1)
9-9: LGTM! Configuration tests updated for package refactoring.Import alias removed and all provider/cluster constant references updated to use the unified client package. Test logic and configuration precedence validation remain unchanged.
Also applies to: 53-53, 118-118, 166-166, 202-202, 370-370, 440-440
src/pkg/agent/tools/deploy.go (1)
12-12: LGTM! Function signature updated for package refactoring.The
ProjectLoadertype reference updated to use the direct client package import. Deployment logic remains unchanged.Also applies to: 24-24
src/pkg/cli/getServices_test.go (1)
11-11: LGTM! Clean import refactoring.The import alias removal and type reference updates are correct and consistent with the PR's package consolidation objectives.
Also applies to: 71-71
src/pkg/agent/tools/removeConfig.go (1)
8-8: LGTM! Function signature correctly updated.The import change and type reference updates align with the package refactoring. The function maintains identical behavior with the new
client.ProjectLoadertype.Also applies to: 21-21, 34-34
src/pkg/mcp/mcp_server.go (1)
9-9: LGTM! Type references correctly updated.The import change and
ProviderIDtype reference update maintain functionality while aligning with the package consolidation. The added comment enhances code documentation.Also applies to: 25-25, 48-48
src/pkg/cli/composeUp.go (1)
32-32: LGTM! Function signature properly updated.The parameter type change from aliased to direct
client.Providerreference is correct and maintains the same functionality.src/pkg/stacks/stacks_test.go (1)
8-8: LGTM! Comprehensive test update.All type references and provider constants are correctly updated throughout the test suite. The changes maintain test functionality while using the new package import path.
Also applies to: 15-22, 48-60, 69-70, 118-120, 205-240, 267-279, 309-310, 340-342, 365-367
src/cmd/cli/command/globals.go (1)
9-10: LGTM! Global configuration correctly updated.The
Clientfield type and initialization logic properly reference the consolidated client package. All default values are correctly updated to use the new package paths.Also applies to: 71-71, 93-94, 98-98
src/pkg/stacks/wizard.go (1)
11-11: LGTM! Wizard logic correctly refactored.All provider-related type references, constants, and function calls are properly updated to use the client package. The wizard's functionality remains unchanged while using the consolidated package structure.
Also applies to: 39-60, 62-71, 84-108, 109-124
src/pkg/agent/tools/logs.go (1)
10-10: LGTM! Function signature correctly updated.The import change and loader parameter type update maintain the same functionality while using the consolidated client package.
Also applies to: 25-25
src/pkg/agent/tools/destroy.go (1)
9-9: LGTM! Clean import alias migration.The refactoring from
cliClientto directclientimport is consistent and correct. The type references and debug messages have been properly updated to reflect the new package structure.Also applies to: 20-20, 33-33
src/cmd/cli/command/estimate.go (1)
7-7: LGTM! Consistent type reference updates.All provider-related constants and types have been correctly migrated from
cliClient.*toclient.*. The changes align perfectly with the package rename objectives.Also applies to: 29-39, 46-46
src/pkg/cli/destroy_test.go (1)
9-9: LGTM! Test updated correctly.The test has been properly updated to use the new
client.PlaygroundProvidertype. The test logic remains unchanged, maintaining test coverage.Also applies to: 57-57
src/pkg/agent/tools/setConfig.go (1)
9-9: LGTM! Refactoring applied consistently.The import and type reference updates match the pattern used across the codebase. The debug message accurately reflects the new package structure.
Also applies to: 21-21, 36-36
src/pkg/cli/preview.go (1)
6-6: LGTM! Function signature updated correctly.The
Previewfunction signature has been properly updated to useclient.FabricClientandclient.Providertypes, maintaining consistency with the package rename.Also applies to: 11-11
src/pkg/agent/tools/listConfig.go (1)
9-9: LGTM! Consistent refactoring applied.The changes follow the established pattern: import alias removal, type reference update, and debug message adjustment. All updates are correct.
Also applies to: 20-20, 33-33
src/pkg/agent/tools/services.go (1)
11-12: LGTM! Import aliases reorganized correctly.The refactoring introduces a new
cliTypesalias for CLI error types while removing the oldcliClientalias. All type references have been updated consistently.Also applies to: 23-23, 47-47
src/cmd/cli/command/commands_test.go (1)
14-14: LGTM! Test imports and types migrated correctly.The test file has been comprehensively updated with both the
clientpackage migration and the AWS driver package rename frompkgtoawsdriver. All type instantiations and assertions have been correctly updated.Also applies to: 18-18, 33-33, 203-203, 263-269, 294-295
src/pkg/agent/tools/estimate.go (4)
8-8: LGTM - Import alias removed.The removal of the
cliClientimport alias aligns with the PR objective to standardize on directclientpackage imports.
20-20: LGTM - Function signature updated.The parameter type correctly updated from
cliClient.ProjectLoadertoclient.ProjectLoader, consistent with the package rename.
29-49: LGTM - Variable renamed for clarity.The variable rename from
clienttofabricimproves code readability by avoiding confusion between the package name and the gRPC client instance. All usages are consistently updated.
36-36: LGTM - Type reference updated.The
ProviderIDtype reference correctly updated to use theclientpackage.src/pkg/agent/tools/provider.go (2)
8-8: LGTM - Import alias removed.Consistent with the package rename refactoring across the codebase.
17-54: LGTM - Type references updated throughout.All type references correctly updated from
cliClient.*toclient.*:
ProviderCreatorinterface method signatureproviderPreparerstruct field- Constructor and method signatures
- Variable declarations
The changes are mechanical and consistent with the package migration.
src/cmd/cli/command/compose.go (5)
15-15: LGTM - Import alias removed.Consistent with the standardization on direct
clientpackage imports.
37-37: LGTM - Constant references updated.References to
client.ProviderDefangandclient.DefaultClustercorrectly updated to reflect the new package structure.
222-256: LGTM - Function signatures and type references updated.The
handleExistingDeploymentsandprintExistingDeploymentsfunctions correctly updated to use*client.AccountInfoandclient.ProviderIDtypes.
300-332: LGTM - Error handling updated.Error types and utility functions correctly migrated:
client.Providerparameter typeclient.PrettyError()callsclient.ErrDeploymentFailederror type
444-722: LGTM - Helper function calls updated.All calls to
client.LoadProjectNameWithFallbackcorrectly updated to use theclientpackage directly.src/pkg/login/login.go (2)
28-77: LGTM - Login implementation updated correctly.The
loginmethod implementation and all public login functions (InteractiveLogin,InteractiveLoginMCP,interactiveLogin) have been correctly updated to:
- Use
fabric client.FabricClientparameter for the gRPC client- Use
cluster stringparameter for the cluster address- Call
client.SaveAccessToken,client.NormalizeHost, andNonInteractiveAgreeToSwith correct parameters
79-114: LGTM - Non-interactive login updated correctly.The
NonInteractiveGitHubLoginfunction properly updated to:
- Accept
fabric client.FabricClientandcluster stringparameters- Use
fabric.Token()for token retrieval- Call
client.SaveAccessToken()andclient.GetWebIdentityTokenFile()with theclusterparametersrc/cmd/cli/command/commands.go (6)
21-21: LGTM - Import alias removed.The
cliClientimport alias has been removed in favor of directclientpackage usage.
60-77: LGTM - Helper function calls updated.Calls to
client.GetExistingToken()andclient.PrettyError()correctly reference theclientpackage.
188-195: LGTM - Provider enumeration updated.The
client.AllProviders()function calls correctly use theclientpackage for provider listing functionality.
799-1058: LGTM - Command implementations updated.All command implementations correctly updated to use:
client.LoadProjectNameWithFallback()client.PrettyError()client.ProviderIDtypeThe changes are consistent across multiple commands (config set, config delete, config list, delete).
1220-1233: LGTM - Type usage in loader configuration.The
ProviderIDtype correctly referenced asclient.ProviderIDin theconfigureLoaderfunction.
1276-1431: LGTM - Provider selection logic updated.All provider-related code correctly migrated:
providerDescriptionmap usesclient.ProviderIDkeys- Provider constants (
client.ProviderDefang,client.ProviderAWS, etc.)- Function signatures use
client.Loaderandclient.Providerclient.AllProviders()andclient.CanIUseProvider()callsinteractiveSelectProviderparameter typesThe refactoring is comprehensive and consistent.
src/pkg/agent/tools/interfaces.go (2)
8-8: LGTM - Import alias removed.Consistent with the package rename across the codebase.
14-32: LGTM - Interface signatures updated.All
CLIInterfacemethod signatures correctly updated to useclient.*types:
*client.GrpcClientfor fabric parametersclient.Providerfor provider parametersclient.Loaderfor loader parametersclient.ProviderIDfor provider ID parametersclient.FabricClientfor fabric client interface parametersThe changes are comprehensive and maintain consistency across all interface methods.
lionello
force-pushed
the
lio/rename-packages
branch
from
59ddb68 to
b81874d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/pkg/agent/tools/removeConfig.go (1)
21-26: Rename local variable to avoid shadowing the imported package.The function signature update is correct, but the local variable
clienton line 23 shadows the importedclientpackage from line 8. While this doesn't cause immediate issues, it reduces code clarity and prevents referencing types from theclientpackage later in the function body.🔎 Suggested fix
func HandleRemoveConfigTool(ctx context.Context, loader client.ProjectLoader, params RemoveConfigParams, cli CLIInterface, ec elicitations.Controller, sc StackConfig) (string, error) { term.Debug("Function invoked: cli.Connect") - client, err := cli.Connect(ctx, sc.Cluster) + fabric, err := cli.Connect(ctx, sc.Cluster) if err != nil { return "", fmt.Errorf("Could not connect: %w", err) } sm := stacks.NewManager(params.WorkingDirectory) - pp := NewProviderPreparer(cli, ec, client, sm) + pp := NewProviderPreparer(cli, ec, fabric, sm)src/pkg/mcp/mcp_server.go (1)
24-28: Consider renaming the struct field to avoid naming collision.The
ToolTrackerstruct has a field namedclient(line 27), which is the same identifier as the imported packageclient(line 9). While this compiles correctly because the contexts differ (struct field vs. package identifier), it can cause confusion when reading the code.Consider renaming the struct field to something like
mcpClientorclientNameto improve clarity.🔎 Proposed refactor
type ToolTracker struct { providerId *client.ProviderID cluster string - client string + mcpClient string }Then update the references accordingly in the file.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
src/cmd/cli/command/cd.go(2 hunks)src/cmd/cli/command/commands.go(21 hunks)src/cmd/cli/command/commands_test.go(5 hunks)src/cmd/cli/command/compose.go(11 hunks)src/cmd/cli/command/compose_test.go(2 hunks)src/cmd/cli/command/estimate.go(2 hunks)src/cmd/cli/command/globals.go(3 hunks)src/cmd/cli/command/globals_test.go(7 hunks)src/cmd/cli/command/mcp.go(2 hunks)src/cmd/cli/command/stack_test.go(4 hunks)src/cmd/cli/command/workspace.go(2 hunks)src/pkg/agent/agent.go(2 hunks)src/pkg/agent/tools/create_aws_stack.go(2 hunks)src/pkg/agent/tools/create_gcp_stack.go(2 hunks)src/pkg/agent/tools/default_tool_cli.go(3 hunks)src/pkg/agent/tools/deploy.go(1 hunks)src/pkg/agent/tools/deploy_test.go(2 hunks)src/pkg/agent/tools/destroy.go(3 hunks)src/pkg/agent/tools/estimate.go(4 hunks)src/pkg/agent/tools/interfaces.go(1 hunks)src/pkg/agent/tools/listConfig.go(3 hunks)src/pkg/agent/tools/logs.go(2 hunks)src/pkg/agent/tools/provider.go(3 hunks)src/pkg/agent/tools/removeConfig.go(3 hunks)src/pkg/agent/tools/services.go(3 hunks)src/pkg/agent/tools/services_test.go(3 hunks)src/pkg/agent/tools/setConfig.go(3 hunks)src/pkg/cli/client/cluster.go(3 hunks)src/pkg/cli/client/cluster_test.go(1 hunks)src/pkg/cli/composeUp.go(1 hunks)src/pkg/cli/configList_test.go(2 hunks)src/pkg/cli/connect.go(1 hunks)src/pkg/cli/destroy_test.go(2 hunks)src/pkg/cli/getServices_test.go(2 hunks)src/pkg/cli/preview.go(1 hunks)src/pkg/cli/tail.go(1 hunks)src/pkg/cli/whoami_test.go(3 hunks)src/pkg/login/login.go(3 hunks)src/pkg/login/login_test.go(4 hunks)src/pkg/mcp/mcp_server.go(3 hunks)src/pkg/mcp/setup_test.go(0 hunks)src/pkg/stacks/stacks_test.go(14 hunks)src/pkg/stacks/wizard.go(5 hunks)src/pkg/stacks/wizard_test.go(26 hunks)src/pkg/track/track.go(1 hunks)
💤 Files with no reviewable changes (1)
- src/pkg/mcp/setup_test.go
🚧 Files skipped from review as they are similar to previous changes (29)
- src/pkg/cli/configList_test.go
- src/cmd/cli/command/stack_test.go
- src/pkg/agent/agent.go
- src/pkg/cli/preview.go
- src/pkg/agent/tools/create_aws_stack.go
- src/pkg/cli/connect.go
- src/pkg/login/login_test.go
- src/pkg/agent/tools/services.go
- src/cmd/cli/command/commands_test.go
- src/pkg/stacks/wizard.go
- src/pkg/cli/composeUp.go
- src/pkg/agent/tools/services_test.go
- src/pkg/stacks/stacks_test.go
- src/pkg/agent/tools/deploy.go
- src/cmd/cli/command/compose.go
- src/pkg/agent/tools/destroy.go
- src/pkg/agent/tools/logs.go
- src/cmd/cli/command/mcp.go
- src/pkg/track/track.go
- src/pkg/agent/tools/create_gcp_stack.go
- src/pkg/cli/destroy_test.go
- src/pkg/cli/tail.go
- src/pkg/cli/getServices_test.go
- src/pkg/agent/tools/interfaces.go
- src/cmd/cli/command/compose_test.go
- src/cmd/cli/command/cd.go
- src/pkg/agent/tools/provider.go
- src/pkg/stacks/wizard_test.go
- src/pkg/agent/tools/setConfig.go
🧰 Additional context used
🧬 Code graph analysis (12)
src/cmd/cli/command/globals_test.go (4)
src/pkg/stacks/stacks.go (1)
StackParameters(17-24)src/pkg/cli/client/provider_id.go (4)
ProviderAuto(13-13)ProviderAWS(15-15)ProviderGCP(17-17)ProviderDefang(14-14)src/pkg/modes/modes.go (2)
Mode(12-12)ModeUnspecified(15-15)src/pkg/cli/client/cluster.go (1)
DefangFabric(16-16)
src/cmd/cli/command/workspace.go (1)
src/pkg/cli/client/cluster.go (1)
GetExistingToken(44-69)
src/cmd/cli/command/globals.go (5)
src/pkg/cli/client/grpc.go (1)
GrpcClient(19-26)src/pkg/cli/client/cluster.go (1)
DefangFabric(16-16)src/pkg/utils.go (1)
GetenvBool(56-59)src/pkg/stacks/stacks.go (1)
StackParameters(17-24)src/pkg/cli/client/provider_id.go (1)
ProviderAuto(13-13)
src/pkg/cli/client/cluster.go (2)
src/pkg/cli/client/state.go (1)
StateDir(15-15)src/pkg/term/colorizer.go (1)
Debug(294-296)
src/cmd/cli/command/estimate.go (4)
src/pkg/cli/client/provider_id.go (4)
ProviderAuto(13-13)ProviderID(10-10)ProviderAWS(15-15)ProviderGCP(17-17)src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)src/pkg/cli/client/client.go (1)
FabricClient(17-44)src/pkg/cli/client/region.go (1)
GetRegion(5-16)
src/pkg/agent/tools/estimate.go (8)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)src/pkg/agent/tools/default_tool_cli.go (1)
StackConfig(19-22)src/pkg/mcp/mcp_server.go (1)
StackConfig(46-46)src/pkg/mcp/tools/tools.go (1)
StackConfig(14-17)src/pkg/cli/connect.go (1)
Connect(16-18)src/pkg/cli/client/provider_id.go (1)
ProviderID(10-10)src/pkg/cli/estimate.go (1)
RunEstimate(23-41)
src/pkg/cli/whoami_test.go (7)
src/pkg/cli/client/playground.go (1)
PlaygroundProvider(17-21)src/pkg/cli/client/client.go (1)
FabricClient(17-44)src/pkg/cli/whoami.go (2)
Whoami(25-59)ShowAccountData(14-23)src/pkg/types/tenant.go (1)
TenantNameOrID(5-5)src/protos/io/defang/v1/fabric.pb.go (4)
Provider(28-28)Provider(66-68)Provider(70-72)Provider(79-81)src/pkg/cli/client/provider.go (1)
Provider(43-69)src/pkg/cli/client/provider_id.go (1)
ProviderDefang(14-14)
src/pkg/mcp/mcp_server.go (1)
src/pkg/cli/client/provider_id.go (1)
ProviderID(10-10)
src/pkg/agent/tools/listConfig.go (4)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)src/pkg/agent/tools/default_tool_cli.go (1)
StackConfig(19-22)src/pkg/mcp/tools/tools.go (1)
StackConfig(14-17)
src/pkg/login/login.go (2)
src/pkg/cli/client/client.go (1)
FabricClient(17-44)src/pkg/cli/client/cluster.go (3)
SaveAccessToken(77-85)NormalizeHost(18-26)GetWebIdentityTokenFile(71-75)
src/pkg/agent/tools/default_tool_cli.go (6)
src/pkg/cli/client/grpc.go (1)
GrpcClient(19-26)src/pkg/cli/connect.go (1)
Connect(16-18)src/pkg/cli/composeUp.go (2)
ComposeUp(32-178)ComposeUpParams(24-29)src/pkg/cli/client/projectName.go (1)
LoadProjectNameWithFallback(10-26)src/pkg/cli/getServices.go (1)
GetServices(28-42)src/pkg/cli/client/client.go (1)
FabricClient(17-44)
src/pkg/agent/tools/removeConfig.go (3)
src/pkg/cli/client/client.go (1)
ProjectLoader(12-15)src/pkg/agent/tools/interfaces.go (1)
CLIInterface(14-32)src/pkg/mcp/tools/tools.go (1)
StackConfig(14-17)
⏰ 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: go-test
🔇 Additional comments (29)
src/cmd/cli/command/workspace.go (2)
10-10: LGTM! Import path updated correctly.The import has been properly updated to reflect the new package location.
19-19: LGTM! Function call updated correctly.The function call has been properly updated to use the new package name while maintaining the same functionality.
src/pkg/agent/tools/removeConfig.go (2)
8-8: Import path change looks good.The removal of the
cliClientalias and direct import of theclientpackage is consistent with the PR's objective to rename packages and merge the cluster package with cli/client.
34-34: Debug message update is consistent with the refactoring.The debug message now references
cli.LoadProjectNameWithFallback, which matches the actual method call on line 35 using thecliparameter. This cosmetic change improves consistency.src/pkg/cli/client/cluster_test.go (2)
11-58: Test logic is sound.The test cases properly cover the token retrieval scenarios with appropriate cleanup and isolation. The logic is correct and comprehensive.
1-1: Package rename fromclustertoclientis complete.All references to the old
clusterpackage have been successfully updated across the codebase. No stale imports or references remain.src/pkg/mcp/mcp_server.go (1)
9-9: LGTM! Import refactoring looks clean.The import path changes and type reference updates are consistent with the PR's objective to rename packages. The added documentation comment for
NewDefangMCPServeris also a nice improvement.Also applies to: 25-25, 48-48
src/pkg/agent/tools/default_tool_cli.go (2)
10-10: LGTM: Import path updated correctly.The removal of the
cliClientalias and direct import ofclientpackage aligns with the PR objectives.
30-115: LGTM: Migration fromcliClienttoclientpackage types is complete and verified.All method signatures have been correctly updated to use the new
clientpackage types (client.GrpcClient,client.Provider,client.ProviderID,client.Loader,client.FabricClient). Parameter naming improvements (e.g.,fabricfor*client.GrpcClient) enhance code clarity. Codebase verification confirms zero remainingcliClientreferences exist across the repository, indicating the migration is complete.src/pkg/login/login.go (5)
23-23: Interface signature updated correctly.The method signature update from
fabric stringtocluster stringaligns with the PR objective to rename parameters.
28-39: Method implementation updated consistently.The parameter renames and package function calls are correctly updated. The call to
client.SaveAccessToken(cluster, token)on line 32 matches the function signature from the relevant code snippets.
51-77: Implementation updated correctly with new package functions.The function implementation is correctly updated:
- Line 58:
client.NormalizeHost(cluster)matches the signature from relevant snippets- Line 61:
client.SaveAccessToken(cluster, token)matches the signature from relevant snippets- All parameter usages are consistent with the new naming
79-114: Function correctly refactored with preserved logic.The parameter renames and function calls are correctly updated:
- Line 86:
fabric.Token(...)correctly uses the renamed parameter- Line 94:
client.SaveAccessToken(cluster, resp.AccessToken)matches the signature- Line 100:
client.GetWebIdentityTokenFile(cluster)matches the signature from relevant snippetsThe AWS web identity token file handling logic is preserved correctly.
43-49: No action required. All call sites use positional arguments and are compatible with the parameter renames.src/pkg/cli/client/cluster.go (1)
1-1: LGTM! Package rename aligns with PR objectives.The package declaration correctly reflects the merge of the cluster package into cli/client.
src/pkg/cli/whoami_test.go (2)
10-10: LGTM! Import path updated correctly.The removal of the import alias and direct use of the client package is consistent with the PR objectives.
42-42: Good variable naming improvement.Renaming the variable from
clienttoproviderbetter reflects that this is aPlaygroundProviderinstance, improving code clarity.src/cmd/cli/command/globals_test.go (2)
9-9: LGTM! Import path updated correctly.The direct import of the client package is consistent with the package refactoring objectives.
53-53: LGTM! Test expectations updated consistently.All references to provider constants and fabric defaults have been correctly updated to use the new
clientpackage namespace throughout the test cases.Also applies to: 118-118, 166-166, 202-202, 370-370, 440-440
src/cmd/cli/command/globals.go (2)
9-10: LGTM! Import reorganization aligns with package refactoring.The addition of the
pkgimport and removal of the alias are consistent with the PR objectives.
94-94: Reconsider: The current implementation is intentional and documented behavior.The
Debugfield reads from theDEFANG_DEBUGenvironment variable viapkg.GetenvBool(), which returnsfalseby default (when the environment variable is not set or is unparseable). This is standard and documented behavior—the environment variable enables debug mode when explicitly set to a truthy value, rather than enabling it "by default." The pattern is used consistently across the codebase and documented in README files, indicating this is intentional design, not a change that contradicts the PR description.Likely an incorrect or invalid review comment.
src/cmd/cli/command/estimate.go (2)
7-7: LGTM! Import path updated correctly.The direct import of the client package is consistent with the package refactoring.
29-47: LGTM! Type references updated consistently.All provider-related types and constants have been correctly updated to use the
clientpackage namespace.src/pkg/agent/tools/deploy_test.go (1)
59-59: LGTM! Parameter naming improved for clarity.Renaming the parameter from
clienttofabricimproves code clarity by better distinguishing the gRPC client instance from the generic term "client." This aligns with the naming conventions used elsewhere in the codebase.Also applies to: 80-80
src/pkg/agent/tools/listConfig.go (2)
9-9: LGTM! Import path updated correctly.The direct import of the client package is consistent with the package refactoring.
20-20: LGTM! Type reference updated correctly.The function signature now correctly uses
client.ProjectLoaderfrom the refactored package.src/pkg/agent/tools/estimate.go (2)
8-8: LGTM! Import path updated correctly.The direct import of the client package is consistent with the package refactoring.
20-20: LGTM! Type references and variable naming updated consistently.The function signature and internal implementation have been correctly updated to use
client.ProjectLoaderandclient.ProviderIDtypes. The variable naming (fabric) is consistent with the pattern used elsewhere in the codebase.Also applies to: 29-29, 34-34, 36-36, 49-49
src/cmd/cli/command/commands.go (1)
21-21: LGTM! The package refactoring is consistently applied.The migration from
cliClientalias to directclientpackage imports is correctly implemented throughout this file. All type references (client.Loader,client.Provider,client.ProviderID), function calls (client.GetExistingToken,client.PrettyError,client.AllProviders, etc.), constants (client.ProviderAuto,client.ProviderAWS, etc.), and function signatures have been consistently updated. Verification confirms no remainingcliClientreferences exist in the codebase, and the import at line 21 is properly converted to a direct import without any alias.
Description
No functional changes, just merging cluster package with cli/client and renaming the imports.
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.