feat!: split startercode policy into branches and issues; apply branch rules during generate#58
Merged
Merged
Conversation
- Migrate legacy `devBranch`, `protectToBranch`, and `protectDevBranchMergeOnly` keys to a new `branches` block for better clarity and separation of concerns. - Introduce `issues` block for issue replication settings, moving from `startercode`. - Update documentation to reflect new configuration structure and provide migration guidance. - Implement branch protection and default settings directly within the `branches` configuration. - Adjust related code in GitLab client to accommodate new structure, ensuring backward compatibility with legacy keys. - Enhance tests to validate new branch protection logic and configuration handling. Co-authored-by: Copilot <copilot@github.com>
Contributor
Coverage |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors assignment policy configuration by splitting legacy startercode responsibilities into dedicated branches and issues blocks, and applies branch rules automatically during repository generation (not only via protect). It also updates CLI show output formatting and provides documentation + migration guidance for the breaking config changes.
Changes:
- Introduce
branches(creation/default/protection/merge-only) andissues(replication) config blocks, with legacystartercode.*keys supported as fallback. - Apply branch creation/default/protection rules during
generatevia a newsyncConfiguredBranchesflow. - Update tests and documentation (including a new migration guide) and improve
showoutput (includingstartercode.additionalBranches).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| gitlab/starterrepo.go | Removes dev-branch/protect behavior from startercode push; tweaks additional-branch push logging/behavior. |
| gitlab/runtime_test.go | Updates runtime test config to use new Branches rules. |
| gitlab/protect.go | Migrates branch protection logic to Branches rules; adds protected-branch update flow. |
| gitlab/protect_contract_test.go | Adapts contract tests to new protection flow (GET+PATCH vs DELETE+POST). |
| gitlab/integration_gitlab_test.go | Updates integration test setup to use Branches configuration. |
| gitlab/generate.go | Applies configured branch rules during generate; switches issue replication to Issues block. |
| gitlab/branches.go | Adds branch sync helper to create branches, set default branch, and invoke protection. |
| docs/workflows.md | Updates workflow examples to use branches / issues. |
| docs/troubleshooting.md | Updates troubleshooting guidance for merge-only behavior under branches. |
| docs/migration.md | Adds migration guide from legacy startercode policy keys to branches / issues. |
| docs/getting-started.md | Updates configuration defaults table and links migration guide. |
| docs/configuration.md | Documents new branches/issues options and clarifies base-branch semantics. |
| docs/advanced.md | Updates advanced example configuration to use branches. |
| config/urls_show_test.go | Adds/updates tests for Show() output (branches, issues, additionalBranches, fmt artifacts). |
| config/types.go | Introduces BranchRule and IssueReplication types; trims Startercode to mapping-only fields. |
| config/show.go | Rewrites Show() output formatting and adds Branches/Issues sections. |
| config/setters_test.go | Updates setter tests to validate SetProtectToBranch populates Branches. |
| config/repo.go | Implements parsing/legacy fallback for branches and issues; makes clone default branch depend on configured default branch. |
| config/repo_test.go | Adds tests for new parsing behavior, legacy fallback, and clone default-branch behavior. |
| config/assignment.go | Wires startercode/branches/issues parsing into GetAssignmentConfig and sets clone default branch accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
55
to
+82
| for _, additionalBranch := range assignmentCfg.Startercode.AdditionalBranches { | ||
| log.Debug().Str("branch", additionalBranch).Msg("pushing additional branch") | ||
|
|
||
| // worktree, err := from.Repo.Worktree() | ||
| // if err != nil { | ||
| // log.Debug().Err(err). | ||
| // Str("branch", additionalBranch). | ||
| // Str("name", project.Name).Str("url", project.SSHURLToRepo). | ||
| // Msg("cannot get worktree") | ||
| // return fmt.Errorf("cannot get worktree: %w", err) | ||
| // } | ||
|
|
||
| // worktree.Checkout(&git.CheckoutOptions{ | ||
| // Branch: plumbing.ReferenceName(additionalBranch), | ||
| // }) | ||
| if additionalBranch == "" { | ||
| continue | ||
| } | ||
|
|
||
| refSpec := config.RefSpec(fmt.Sprintf("+refs/remotes/origin/%s:refs/heads/%s", additionalBranch, additionalBranch)) | ||
|
|
||
| log.Debug(). | ||
| Str("refSpec", string(refSpec)). | ||
| Str("name", project.Name). | ||
| Str("toURL", project.SSHURLToRepo). | ||
| Str("branch", additionalBranch). | ||
| Msg("pushing additional startercode branch") | ||
|
|
||
| pushOpts := &git.PushOptions{ | ||
| RemoteName: remote.Config().Name, | ||
| RefSpecs: []config.RefSpec{refSpec}, | ||
| Auth: from.Auth, | ||
| } | ||
| err = from.Repo.Push(pushOpts) | ||
| if err != nil { | ||
| log.Debug().Err(err). | ||
| log.Warn().Err(err). | ||
| Str("branch", additionalBranch). | ||
| Str("refspec", refSpec.String()). | ||
| Str("name", project.Name).Str("url", project.SSHURLToRepo). | ||
| Msg("cannot push to remote") | ||
| return fmt.Errorf("cannot push to remote: %w", err) | ||
| Str("refSpec", refSpec.String()). | ||
| Str("name", project.Name). | ||
| Str("url", project.SSHURLToRepo). | ||
| Msg("cannot push additional branch to remote, continuing with other setup steps") | ||
| continue |
There was a problem hiding this comment.
Pushing an explicitly configured startercode.additionalBranches branch is now best-effort (Warn + continue). If the push fails (e.g., branch missing in starter), generation continues and later branch creation may fall back to baseBranch content, leaving a branch that exists but has the wrong history/content. Consider failing the generate step (or at least returning an aggregated error) when a configured additional branch cannot be mirrored, so misconfigurations don’t silently produce incorrect repositories.
Comment on lines
+164
to
+175
| if generated && len(assignmentCfg.Branches) > 0 { | ||
| baseBranch := defaultBranchName(assignmentCfg.Branches, "main") | ||
| if assignmentCfg.Startercode != nil { | ||
| baseBranch = assignmentCfg.Startercode.ToBranch | ||
| } else if assignmentCfg.Seeder != nil { | ||
| baseBranch = assignmentCfg.Seeder.ToBranch | ||
| } | ||
|
|
||
| if err := c.syncConfiguredBranches(assignmentCfg, project, baseBranch); err != nil { | ||
| log.Debug().Err(err).Str("project", project.Name).Msg("cannot apply configured branch rules") | ||
| } | ||
| } |
There was a problem hiding this comment.
syncConfiguredBranches is called for any generated project with a non-empty Branches config, even when neither startercode nor seeder ran. Newly created GitLab projects are empty by default (no ref), so creating branches from baseBranch and setting DefaultBranch will fail because baseBranch doesn’t exist yet. Consider guarding this block to only run after startercode/seeder created the base branch, or initialize the project with an initial commit/README before applying branch rules.
Comment on lines
+186
to
+193
| func isProtectedBranchNotFoundError(err error) bool { | ||
| if err == nil { | ||
| return false | ||
| } | ||
|
|
||
| msg := strings.ToLower(err.Error()) | ||
| return strings.Contains(msg, "404") || strings.Contains(msg, "not found") | ||
| } |
There was a problem hiding this comment.
isProtectedBranchNotFoundError detects 404s by substring matching on err.Error(). This is brittle and can misclassify unrelated errors (different wording/localization, or other statuses containing “not found”). Prefer checking the GitLab API response status code (the *Response returned by GetProtectedBranch) or using errors.As to inspect a structured error type, and only treating an actual 404 as the “not protected yet” case.
Comment on lines
+67
to
+70
| func isBranchAlreadyExistsError(err error) bool { | ||
| msg := strings.ToLower(err.Error()) | ||
| return strings.Contains(msg, "already exists") || strings.Contains(msg, "has already been taken") | ||
| } |
There was a problem hiding this comment.
isBranchAlreadyExistsError relies on substring matching against err.Error() to detect the “branch already exists” case. This is fragile (message text can change) and risks either masking real errors or failing to ignore the specific “already exists” condition. Consider using the GitLab API response/status code (e.g., 400 with the known validation error) or a structured error type from the client library instead of parsing strings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
branchesandissuesconfig blocks, independent fromstartercodestartercodefocused on source/target branch mappingstartercode.additionalBranchesmirror behavior (starter/x -> repo/x)generate(not only viaprotect)showoutput formatting and includestartercode.additionalBranchesshowoutputBreaking Changes
startercodeintobranchesandissuesstartercode.*policy keys are now compatibility fallback and should be migratedstartercode.additionalBranchesare initialized fromtoBranchstateMigration Notes
startercode:url,fromBranch,toBranch,additionalBranchesprotect,mergeOnly,default) tobranchesissues.replicateFromStartercode/issues.issueNumbersglabs protectto apply branch rules to already existing repositoriesBREAKING CHANGE: branch and issue policy moved from startercode to branches/issues.