refactor: separate scope/project commands, eliminate shared globals#50
Conversation
Architectural refactor for v0.3.7 — decouples scope and project commands, eliminates ~18 shared mutable package-level variables, and fixes orchestrator commands (full, init) to properly chain phases. ## Scope command (configure_scopes.go) - Accepts ScopeOpts struct instead of reading package globals - Only PUTs scopes to connections — no longer creates projects (#35) - Removed project-related flags (--project-name, --cron, --time-after, etc.) - After scoping, prints hint to run 'configure project' ## Project command (configure_projects.go) - Accepts ProjectOpts struct instead of reading package globals - Uses new ListScopes API to discover existing scopes (#37) - No longer PUTs scopes — references existing ones in blueprints - Removed fall-through to runConfigureScopes in flag mode (#41) - Owns finalizeProject, ensureProjectWithFlags, triggerAndPoll ## Full command (configure_full.go) - 3-phase flow: connections -> scopes -> project - Passes DevLake client/state from Phase 1 to Phase 3 (#42) - Each phase uses its own opts struct, no shared mutable state ## Init wizard (init.go) - 4-phase flow: deploy -> connections -> scopes -> project - Phase 4 calls runConfigureProjects (not runConfigureScopes) (#40) - Passes connection results via opts structs ## Client (internal/devlake/client.go) - Added ListScopes method: GET /plugins/{plugin}/connections/{id}/scopes (#46) ## Types (internal/devlake/types.go) - Added ScopeListEntry and ScopeListResponse types (#46) ## Tests (configure_scopes_test.go) - Tests use ScopeOpts struct — no global mutation (#45) - Previously-failing TestRunConfigureScopes_PluginFlag now passes Closes #35, #37, #39, #40, #41, #42, #45, #46
There was a problem hiding this comment.
Pull request overview
This PR implements a major architectural refactoring that successfully decouples scope and project commands by eliminating ~18 shared mutable package-level variables and establishing clear separation of concerns.
Changes:
- Introduces opts structs (ScopeOpts, ProjectOpts) to replace package-level scope* variables, making commands independently testable
- Separates scope command (now only PUTs scopes) from project command (now uses ListScopes API to reference existing scopes)
- Fixes orchestrator commands (full, init) to properly chain phases and pass client/state forward to avoid redundant discovery
- Adds ListScopes client method and supporting types for the project command to discover existing scopes
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/devlake/types.go | Adds ScopeListEntry and ScopeListResponse types for GET /scopes API |
| internal/devlake/client.go | Adds ListScopes method to query existing scopes on connections |
| cmd/configure_scopes_test.go | Updates tests to use ScopeOpts struct instead of mutating globals |
| cmd/configure_scopes.go | Refactors to accept ScopeOpts, removes project creation, only PUTs scopes |
| cmd/configure_projects.go | Refactors to accept ProjectOpts, uses ListScopes instead of scoping connections |
| cmd/configure_full.go | Implements 3-phase flow (connections→scopes→project) passing client/state forward |
| cmd/init.go | Implements 4-phase flow, now correctly calls runConfigureProjects in Phase 4 |
Comments suppressed due to low confidence (14)
cmd/init.go:135
- According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add
fmt.Println()after line 134 to properly separate the banner from the following content.
This issue also appears in the following locations of the same file:
- line 191
- line 91
- line 67
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551 PHASE 3: Configure Scopes \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")
cmd/init.go:194
- According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add
fmt.Println()after line 193 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551 PHASE 4: Project Setup \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")
cmd/configure_full.go:105
- According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add
fmt.Println()after line 104 to properly separate the banner from the following content.
This issue also appears in the following locations of the same file:
- line 168
- line 113
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551 PHASE 1: Configure Connections \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")
cmd/configure_full.go:171
- According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add
fmt.Println()after line 170 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551 PHASE 3: Project Setup \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")
cmd/init.go:94
- According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add
fmt.Println()after line 93 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551 PHASE 2: Configure Connections \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")
cmd/configure_scopes.go:276
- According to the terminal output standards, separators need blank lines before AND after them. Currently there's a
\nprefix before the opening separator (line 267) but no blank line after the closing separator (line 275). The pattern should be: blank line before opening separator → content → closing separator → blank line. Addfmt.Println()after line 275 to properly separate the summary section from the "Next step" hint.
fmt.Println("\n" + strings.Repeat("\u2500", 50))
fmt.Println("\u2705 Scopes configured successfully!")
if !opts.SkipGitHub {
fmt.Printf(" GitHub connection %d: scopes added\n", ghConnID)
}
if !opts.SkipCopilot && copilotConnID > 0 {
fmt.Printf(" Copilot connection %d: scope added\n", copilotConnID)
}
fmt.Println(strings.Repeat("\u2500", 50))
fmt.Println("\nNext step:")
cmd/init.go:144
- According to the terminal output standards, there should be a blank line before interactive prompts. Add
fmt.Println()before line 144 to properly separate the prompt from the preceding output.
if !prompt.Confirm("Use these defaults?") {
cmd/configure_projects.go:404
- According to the terminal output standards, separators need blank lines before AND after them. Currently there's a
\nprefix before the opening separator (line 394) but no blank line after the closing separator (line 403). Addfmt.Println()after line 403 to provide proper spacing.
fmt.Println("\n" + strings.Repeat("\u2500", 50))
fmt.Println("\u2705 Project configured successfully!")
fmt.Printf(" Project: %s\n", opts.ProjectName)
if len(opts.Repos) > 0 {
fmt.Printf(" Repos: %s\n", strings.Join(opts.Repos, ", "))
}
if opts.HasCopilot {
fmt.Printf(" Copilot: %s\n", opts.Org)
}
fmt.Println(strings.Repeat("\u2500", 50))
cmd/configure_full.go:116
- According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add
fmt.Println()after line 115 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551 PHASE 2: Configure Scopes \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")
cmd/init.go:70
- According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add
fmt.Println()after line 69 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551 PHASE 1: Deploy DevLake \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")
cmd/configure_scopes.go:36
- The
ScopeOptsstruct contains several unused project-related fields:ProjectName,TimeAfter,Cron,SkipSync,Wait, andTimeout. Since the scope command no longer creates projects or triggers syncs (per the architectural separation in this PR), these fields are dead code and should be removed. Keep only the fields actually used by the scope command:Org,Enterprise,Plugin,Repos,ReposFile,GHConnID,CopilotConnID,DeployPattern,ProdPattern,IncidentLabel,SkipCopilot, andSkipGitHub.
type ScopeOpts struct {
Org string
Enterprise string
Plugin string
Repos string
ReposFile string
GHConnID int
CopilotConnID int
ProjectName string
DeployPattern string
ProdPattern string
IncidentLabel string
TimeAfter string
Cron string
SkipSync bool
SkipCopilot bool
SkipGitHub bool
Wait bool
Timeout string
}
cmd/init.go:212
- According to the terminal output standards, completion banners need blank lines after them. Currently there's a blank line before the opening banner (line 209) but no blank line after the closing banner (line 211). The standard requires
fmt.Println()after the closing banner. Line 212 already has this, so this banner correctly follows the standard.
fmt.Println("\n\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550")
fmt.Println(" \u2705 DevLake is ready!")
fmt.Println("\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550")
fmt.Println()
cmd/configure_full.go:276
- According to the terminal output standards, separators need blank lines before AND after them. Currently there's a
\nprefix before the opening separator (line 266) but no blank line after the closing separator (line 275). Addfmt.Println()after line 275 to provide proper spacing before the return statement.
fmt.Println("\n" + strings.Repeat("\u2500", 50))
fmt.Println("\u2705 Connections configured successfully!")
for _, r := range results {
name := r.Plugin
if def := FindConnectionDef(r.Plugin); def != nil {
name = def.DisplayName
}
fmt.Printf(" %-18s ID=%d %q\n", name, r.ConnectionID, r.Name)
}
fmt.Println(strings.Repeat("\u2500", 50))
cmd/configure_full.go:193
- According to the terminal output standards, completion banners need blank lines after them. Line 193 already provides this with
fmt.Println(). This banner correctly follows the standard.
fmt.Println("\n\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550")
fmt.Println(" \u2705 Full configuration complete!")
fmt.Println("\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550")
fmt.Println()
Summary
Architectural refactor that decouples scope and project commands, eliminates ~18 shared mutable package-level variables, and fixes orchestrator commands.
This is the core of v0.3.7 -- addresses the root cause of the scope/project entanglement.
Key Changes
Scope command -- scope-only (#35, #39)
ScopeOptsstruct instead of reading package globals--project-name,--cron,--time-after, etc.)configure projectafter scopingProject command -- project-only (#37, #41)
ProjectOptsstruct instead of reading package globalsListScopesAPI to discover existing scopes on connectionsrunConfigureScopeswhen connection IDs passed as flagsFull command -- clean phase chaining (#42)
Init wizard -- fixed delegation (#40)
runConfigureProjects(notrunConfigureScopes)Client API (#46)
ListScopesmethod:GET /plugins/{plugin}/connections/{id}/scopesScopeListEntryandScopeListResponsetypesTests (#45)
ScopeOptsstruct -- zero global mutationTestRunConfigureScopes_PluginFlagnow passesgo test ./...greenTesting
go build-- cleango test ./...-- ALL PASS (including previously-failing test)Closes #35, #37, #39, #40, #41, #42, #45, #46