diff --git a/tools/fork-cli/INTERNAL_CONTRIB.md b/tools/fork-cli/INTERNAL_CONTRIB.md index 4e2f267240dea..ab849f14bc032 100644 --- a/tools/fork-cli/INTERNAL_CONTRIB.md +++ b/tools/fork-cli/INTERNAL_CONTRIB.md @@ -40,7 +40,7 @@ Our fork maintains several important branches: # If you have changes: git stash push -m "WIP before sync" # or - git add . && git commit -m "WIP: save local changes" + git add . && git commit -m "chore(WIP): save local changes" ``` 2. **Sync our fork with upstream**: @@ -77,6 +77,9 @@ Our fork maintains several important branches: 3. **Make changes and create PRs**: - Make changes, commit, and push your feature branch + - Skim through argocd [contribution guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/code-contributions/) + - run `make pre-commit-local` to run pre commit checks (it's slow) + - make sure **all** commit messages follow the pattern `^(feat|fix|docs|test|ci|chore)!?(\\(.*\\))?!?:.*` - Create PRs against the development branch (not upstream!) - Use the `gh` CLI for convenience: ```shell diff --git a/tools/fork-cli/main.go b/tools/fork-cli/main.go index 9a2ed9df39f55..8d1d54c3a071b 100644 --- a/tools/fork-cli/main.go +++ b/tools/fork-cli/main.go @@ -314,7 +314,7 @@ func promoteFixCmd(args []string, runner CommandRunner) int { return 1 } - baseRelease := parts[2] // e.g. v2.9.14 + baseRelease := strings.Join(parts[:3], "/") // e.g. skyscanner-internal/develop/v2.9.14 // 3) Compute merge-base & commit-range gb := strings.TrimSpace(runner.RunAndCaptureOrExit("git", "merge-base", *fixBranch, baseRelease)) @@ -514,8 +514,8 @@ func workOnCmd(args []string, runner CommandRunner) int { runner.RunOrExit("git", "push", "-u", "origin", featureBranch) // Create PR automatically - title := fmt.Sprintf("feat: %s", *suffix) - body := fmt.Sprintf("Automated PR created via fork-cli\n\nUpdates VERSION to %s", newVersion) + title := "feat: " + *suffix + body := "Automated PR created via fork-cli\n\nUpdates VERSION to " + newVersion runner.RunOrExit("gh", "pr", "create", "--base", *devBranch, "--title", title, "--body", body) fmt.Println("āœ… PR created successfully!") @@ -528,7 +528,7 @@ func workOnCmd(args []string, runner CommandRunner) int { } func printConflictMessage(w io.Writer, releaseTag, fixBranch, proposal string) { - cmd := fmt.Sprintf("go run tools/fork-cli/main.go promote-fix --fix-branch=\"%s\" --proposal-branch=\"%s\"", + cmd := fmt.Sprintf("go run tools/fork-cli/main.go promote-fix --fix-branch=%q --proposal-branch=%q", fixBranch, proposal) fmt.Fprintf(w, "\nāŒ Conflict detected during promotion of release %s.\n", releaseTag) fmt.Fprintln(w, "Please re-run this command locally to resolve interactively:") diff --git a/tools/fork-cli/main_test.go b/tools/fork-cli/main_test.go index 8eac0c8c8430d..71bdc381d76d1 100644 --- a/tools/fork-cli/main_test.go +++ b/tools/fork-cli/main_test.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "errors" "fmt" "os" @@ -19,7 +18,6 @@ type MockRunner struct { exitCalls []string branchesExist map[string]bool tagsExist map[string]bool - output *bytes.Buffer cmdIndex int hasUncommittedChanges bool branchUpToDate map[string]bool @@ -39,7 +37,6 @@ func NewMockRunner(t *testing.T) *MockRunner { expectedCmds: []expectedCmd{}, branchesExist: make(map[string]bool), tagsExist: make(map[string]bool), - output: &bytes.Buffer{}, branchUpToDate: make(map[string]bool), } } @@ -154,7 +151,7 @@ func (m *MockRunner) HasUncommittedChanges() bool { func (m *MockRunner) IsBranchUpToDate(localBranch, remoteBranch string) bool { upToDate, ok := m.branchUpToDate[localBranch] if !ok { - m.t.Logf("Branch up-to-date check not mocked for: %s", localBranch) + m.t.Logf("Branch up-to-date check not mocked for local branch %s and remote branch %s", localBranch, remoteBranch) return false } return upToDate @@ -269,8 +266,9 @@ func TestPromoteFixCmd(t *testing.T) { proposalFull := "skyscanner-contrib/proposal/" + proposalBranch mock.SetBranchUpToDate("skyscanner-contrib/master", true) + + // Expected command sequence: mock.ExpectCommand("git", "fetch", "origin", "skyscanner-contrib/master:skyscanner-contrib/master") - mock.ExpectCommand("git", "fetch", "origin", fixBranch+":"+fixBranch) mock.ExpectCommand("git", "merge-base", fixBranch, "skyscanner-internal/develop/v2.14.9").WithOutput("abcdef123456") mock.ExpectCommand("git", "checkout", "skyscanner-contrib/master") mock.SetBranchExists(proposalFull, false) @@ -290,7 +288,6 @@ func TestPromoteFixCmd(t *testing.T) { mock.SetBranchUpToDate("skyscanner-contrib/master", false) mock.ExpectCommand("git", "fetch", "origin", "skyscanner-contrib/master:skyscanner-contrib/master") - mock.ExpectCommand("git", "fetch", "origin", fixBranch+":"+fixBranch) args := []string{"--fix-branch=" + fixBranch, "--proposal-branch=" + proposalBranch} exitCode := promoteFixCmd(args, mock) @@ -306,10 +303,8 @@ func TestPromoteFixCmd(t *testing.T) { mock.SetBranchUpToDate("skyscanner-contrib/master", true) mock.ExpectCommand("git", "fetch", "origin", "skyscanner-contrib/master:skyscanner-contrib/master") - mock.ExpectCommand("git", "fetch", "origin", fixBranch+":"+fixBranch) mock.ExpectCommand("git", "merge-base", fixBranch, "skyscanner-internal/develop/v2.14.9").WithOutput("abcdef123456") mock.ExpectCommand("git", "checkout", "skyscanner-contrib/master") - mock.SetBranchExists(proposalFull, false) mock.ExpectCommand("git", "checkout", "-b", proposalFull) mock.ExpectCommand("git", "cherry-pick", "--keep-redundant-commits", "abcdef123456.."+fixBranch).WithError(errors.New("conflict")) @@ -337,8 +332,8 @@ func TestSyncForkCmd(t *testing.T) { // Set test environment oldToken := os.Getenv("GITHUB_TOKEN") - os.Setenv("GITHUB_TOKEN", "test-token") - defer os.Setenv("GITHUB_TOKEN", oldToken) + t.Setenv("GITHUB_TOKEN", "test-token") + defer t.Setenv("GITHUB_TOKEN", oldToken) // Run the command exitCode := syncForkCmd([]string{}, mock) @@ -453,10 +448,17 @@ func TestWorkOnCmd(t *testing.T) { mock := NewMockRunner(t) devBranch := "skyscanner-internal/develop/v2.14.9/fix-issue-123" suffix := "add-logging" + featureBranch := fmt.Sprintf("%s-%s", devBranch, suffix) - mock.ExpectCommand("git", "fetch", "origin", devBranch+":"+devBranch) mock.ExpectCommand("git", "checkout", devBranch) - mock.ExpectCommand("git", "checkout", "-b", "feature/add-logging") + mock.ExpectCommand("git", "checkout", "-b", featureBranch) + mock.ExpectCommand("cat", "VERSION").WithOutput("v2.14.9") + mock.ExpectCommand("sh", "-c", "echo 'v2.14.9-add-logging' > VERSION") + mock.ExpectCommand("git", "add", "VERSION") + mock.ExpectCommand("git", "commit", "-m", "feat: add-logging\n\nUpdate VERSION to v2.14.9-add-logging") + mock.ExpectCommand("gh", "repo", "set-default", "Skyscanner/argo-cd") + mock.ExpectCommand("git", "push", "-u", "origin", "skyscanner-internal/develop/v2.14.9/fix-issue-123-add-logging") + mock.ExpectCommand("gh", "pr", "create", "--base", "skyscanner-internal/develop/v2.14.9/fix-issue-123", "--title", "feat: add-logging", "--body", "Automated PR created via fork-cli\n\nUpdates VERSION to v2.14.9-add-logging") exitCode := workOnCmd([]string{"--dev-branch=" + devBranch, "--suffix=" + suffix}, mock) assert.Equal(t, 0, exitCode)