Add --force flag to connection delete and scope delete#107
Conversation
Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a --force flag to configure connection delete and configure scope delete commands to allow skipping the interactive confirmation prompt in CI/CD or scripted environments. It follows the identical pattern already established by cleanup.go's cleanupForce flag.
Changes:
configure_connection_delete.go— addsconnDeleteForce boolpackage-level var, registers--forceflag viainit(), and short-circuitsprompt.Confirmwhen set.configure_scope_delete.go— same pattern:scopeDeleteForce bool,--forceflag, confirmation bypassed when true.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
cmd/configure_connection_delete.go |
Adds connDeleteForce flag variable, registers --force with Cobra, and short-circuits prompt.Confirm when the flag is set |
cmd/configure_scope_delete.go |
Adds scopeDeleteForce flag variable, registers --force with Cobra, and short-circuits prompt.Confirm when the flag is set |
| cmd.Flags().StringVar(&scopeDeletePlugin, "plugin", "", fmt.Sprintf("Plugin of the connection (%s)", strings.Join(availablePluginSlugs(), ", "))) | ||
| cmd.Flags().IntVar(&scopeDeleteConnID, "connection-id", 0, "Connection ID") | ||
| cmd.Flags().StringVar(&scopeDeleteScopeID, "scope-id", "", "Scope ID to delete") | ||
| cmd.Flags().BoolVar(&scopeDeleteForce, "force", false, "Skip confirmation prompt") |
There was a problem hiding this comment.
The --force flag is not reflected anywhere the command is documented. Two places need updating: (1) The Long description's examples block (lines 28–30) should include a --force example (e.g., gh devlake configure scope delete --plugin github --connection-id 1 --scope-id 12345678 --force), consistent with how cleanup.go shows gh devlake cleanup --azure --force in its own Long string. (2) docs/configure-scope.md (lines 163–167) lists the flags table for this command but --force is absent; it should be added with description "Skip confirmation prompt", and a matching --force example should appear in the examples block.
| func init() { | ||
| deleteConnectionCmd.Flags().StringVar(&connDeletePlugin, "plugin", "", "Plugin of the connection to delete") | ||
| deleteConnectionCmd.Flags().IntVar(&connDeleteID, "id", 0, "ID of the connection to delete") | ||
| deleteConnectionCmd.Flags().BoolVar(&connDeleteForce, "force", false, "Skip confirmation prompt") |
There was a problem hiding this comment.
The --force flag is not reflected anywhere the command is documented. Two places need updating: (1) The Long description's examples block should include a --force example (e.g., gh devlake configure connection delete --plugin github --id 3 --force), consistent with how cleanup.go shows gh devlake cleanup --azure --force in its Long string. (2) docs/configure-connection.md (lines 211–214) lists the flags table for this command but --force is absent; it should be added with description "Skip confirmation prompt", and an example with --force should appear in the examples block.
| fmt.Println(" Any scopes and blueprint references for this connection will also be lost.") | ||
| fmt.Println() | ||
| if !prompt.Confirm("Are you sure you want to delete this connection?") { | ||
| if !connDeleteForce && !prompt.Confirm("Are you sure you want to delete this connection?") { |
There was a problem hiding this comment.
The existing test file cmd/configure_connection_delete_test.go already covers related runDeleteConnection behaviours (unknown plugin, partial flags). It does not include a test verifying that when connDeleteForce is set to true the function skips the confirmation prompt. Given the other tests in this file directly exercise runDeleteConnection with specific flag combinations, a test like TestRunDeleteConnection_ForceSkipsConfirm is missing. Without it, a regression that re-introduces the blocking prompt in the --force path would go undetected.
|
@copilot Please address all 3 review comments:
|
…dd force test Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
All three addressed in b245519:
|
|
|
||
| fmt.Println() | ||
| if !prompt.Confirm("Are you sure you want to delete this scope?") { | ||
| if !scopeDeleteForce && !prompt.Confirm("Are you sure you want to delete this scope?") { |
There was a problem hiding this comment.
The PR adds --force support to runScopeDelete but provides no test coverage for that path. A parallel test to TestRunDeleteConnection_ForceSkipsConfirm — which verifies that with scopeDeleteForce = true and all three flags set, the function proceeds past confirmation to DevLake discovery (returning a discovery error, not nil, and not a validation error) — is missing. Since configure_connection_delete_test.go already tests the analogous path, a configure_scope_delete_test.go (or tests within configure_scopes_test.go) should cover the same behavior for scope delete.
|
@copilot One more fix: add a parallel test |
…e path Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
Added |
configure connection deleteandconfigure scope deletealways invokeprompt.Confirm, making them hang in non-interactive environments (CI/CD, scripts).Changes
configure_connection_delete.go— addsconnDeleteForce boolpackage-level var, registers--forceflag, short-circuits confirmation when set;Longdescription updated with--forceexampleconfigure_scope_delete.go— same pattern:scopeDeleteForce bool,--forceflag, skipsprompt.Confirmwhen true;Longdescription updated with--forceexampledocs/configure-connection.md—--forceadded to flags table and examples blockdocs/configure-scope.md—--forceadded to flags table and examples blockcmd/configure_connection_delete_test.go— addedTestRunDeleteConnection_ForceSkipsConfirmto verify the force path proceeds past confirmationcmd/configure_scope_delete_test.go— addedTestRunScopeDelete_ForceSkipsConfirmto verify the scope delete force path proceeds past confirmation to DevLake discoveryFollows the existing
cleanupForcepattern fromcleanup.go.Without
--force, both commands behave exactly as before.Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.