diff --git a/gitcmds/dev.go b/gitcmds/dev.go index dc5fc84..5fe345c 100644 --- a/gitcmds/dev.go +++ b/gitcmds/dev.go @@ -18,11 +18,7 @@ import ( ) // CreateDevBranch creates dev branch and pushes it to origin -// Parameters: -// branch - branch name -// notes - notes for branch -// checkRemoteBranchExistence - if true, checks if a branch already exists in remote -func CreateDevBranch(wd, branchName, mainBranch string, notes []string, checkRemoteBranchExistence bool) error { +func CreateDevBranch(wd, branchName, mainBranch string, notes []string) error { branchName = normalizeBranchName(branchName) if branchName == "" { return errors.New("branch name is empty after normalization") @@ -45,24 +41,6 @@ func CreateDevBranch(wd, branchName, mainBranch string, notes []string, checkRem return err } - if checkRemoteBranchExistence { - // check if a branch already exists in remote - stdout, stderr, err := new(exec.PipedExec). - Command(git, "ls-remote", "--heads", "origin", branchName). - WorkingDir(wd). - RunToStrings() - if err != nil { - logger.Verbose(stderr) - - return err - } - logger.Verbose(stdout) - - if len(stdout) > 0 { - return fmt.Errorf("branch %s already exists in origin remote", branchName) - } - } - // Create new branch from main err = new(exec.PipedExec). Command(git, "checkout", "-B", branchName). diff --git a/gitcmds/github.go b/gitcmds/github.go index 0459e2a..3111962 100644 --- a/gitcmds/github.go +++ b/gitcmds/github.go @@ -19,15 +19,16 @@ import ( "github.com/untillpro/qs/utils" ) -// CreateGithubLinkToIssue create a link between an upstream GitHub issue and the dev branch -func CreateGithubLinkToIssue(wd, parentRepo, githubIssueURL string, issueNumber int, args ...string) (branch string, notes []string, err error) { +// LinkBranchToGithubIssue links an existing remote branch to a GitHub issue and prepares notes. +// The branch must already exist on the remote before calling this function. +func LinkBranchToGithubIssue(wd, parentRepo, githubIssueURL string, issueNumber int, branchName string, args ...string) (notes []string, err error) { repo, org, err := GetRepoAndOrgName(wd) if err != nil { - return "", nil, fmt.Errorf("GetRepoAndOrgName failed: %w", err) + return nil, fmt.Errorf("GetRepoAndOrgName failed: %w", err) } if len(repo) == 0 { - return "", nil, errors.New(repoNotFound) + return nil, errors.New(repoNotFound) } strIssueNum := strconv.Itoa(issueNumber) @@ -49,40 +50,16 @@ func CreateGithubLinkToIssue(wd, parentRepo, githubIssueURL string, issueNumber logger.Verbose(stderr) if len(stderr) > 0 { - return "", nil, errors.New(stderr) + return nil, errors.New(stderr) } - return "", nil, fmt.Errorf("failed to set default repo: %w", err) + return nil, fmt.Errorf("failed to set default repo: %w", err) } printLn(stdout) - branchName, err := buildDevBranchName(githubIssueURL) - if err != nil { - return "", nil, err - } - - // check if a branch already exists in remote - stdout, stderr, err = new(exec.PipedExec). - Command(git, "ls-remote", "--heads", origin, branch). - WorkingDir(wd). - RunToStrings() - if err != nil { - logger.Verbose(stderr) - - if len(stderr) > 0 { - return "", nil, errors.New(stderr) - } - - return "", nil, fmt.Errorf("failed to check if branch exists in origin remote: %w", err) - } - - if len(stdout) > 0 { - return "", nil, fmt.Errorf("branch %s already exists in origin remote", branch) - } - mainBranch, err := GetMainBranch(wd) if err != nil { - return "", nil, fmt.Errorf(errMsgFailedToGetMainBranch, err) + return nil, fmt.Errorf(errMsgFailedToGetMainBranch, err) } stdout, stderr, err = new(exec.PipedExec). @@ -93,40 +70,16 @@ func CreateGithubLinkToIssue(wd, parentRepo, githubIssueURL string, issueNumber logger.Verbose(stderr) if len(stderr) > 0 { - return "", nil, errors.New(stderr) + return nil, errors.New(stderr) } - return "", nil, fmt.Errorf("failed to create development branch for issue: %w", err) - } // delay to ensure branch is created + return nil, fmt.Errorf("failed to link branch to issue: %w", err) + } logger.Verbose(stdout) utils.DelayIfTest() - branch = strings.TrimSpace(stdout) - segments := strings.Split(branch, slash) - branch = segments[len(segments)-1] - - if len(branch) == 0 { - return "", nil, errors.New("can not create branch for issue") - } - // old-style notes - issueName, err := GetGithubIssueNameByNumber(strIssueNum, parentRepo) - if err != nil { - return "", nil, err - } - - comment := IssuePRTtilePrefix + " '" + issueName + "' " - body := "" - if len(issueName) > 0 { - body = IssueSign + strIssueNum + oneSpace + issueName - } - // Prepare new notes with issue name as description - notesObj, err := notesPkg.Serialize(githubIssueURL, "", notesPkg.BranchTypeDev, issueName) - if err != nil { - return "", nil, err - } - - return branch, []string{comment, body, notesObj}, nil + return nil, nil } func GetGithubIssueRepoFromURL(url string) (repoName string) { @@ -147,67 +100,62 @@ func GetGithubIssueRepoFromURL(url string) (repoName string) { return } -func buildDevBranchName(issueURL string) (string, error) { - // Extract issue number from URL +func BuildDevBranchName(issueURL string) (string, []string, error) { parts := strings.Split(issueURL, slash) if len(parts) < 2 { - return "", fmt.Errorf("invalid issue URL format: %s", issueURL) + return "", nil, fmt.Errorf("invalid issue URL format: %s", issueURL) } issueNumber := parts[len(parts)-1] - // Extract owner and repo from URL repoURL := strings.Split(issueURL, "/issues/")[0] urlParts := strings.Split(repoURL, slash) if len(urlParts) < 5 { //nolint:revive - return "", fmt.Errorf("invalid GitHub URL format: %s", repoURL) + return "", nil, fmt.Errorf("invalid GitHub URL format: %s", repoURL) } owner := urlParts[3] //nolint:revive repo := urlParts[4] //nolint:revive - // Use gh CLI to get issue title stdout, stderr, err := new(exec.PipedExec). Command("gh", "issue", "view", issueNumber, "--repo", fmt.Sprintf("%s/%s", owner, repo), "--json", "title"). RunToStrings() - if err != nil { logger.Verbose(stderr) - if len(stderr) > 0 { - return "", errors.New(stderr) + return "", nil, errors.New(stderr) } - - return "", fmt.Errorf("failed to get issue title: %w", err) + return "", nil, fmt.Errorf("failed to get issue title: %w", err) } logger.Verbose(stdout) - // Parse JSON response var issueData struct { Title string `json:"title"` } - if err := json.Unmarshal([]byte(stdout), &issueData); err != nil { - return "", fmt.Errorf("failed to parse issue data: %w", err) + return "", nil, fmt.Errorf("failed to parse issue data: %w", err) } - // Create kebab-case version of the title kebabTitle := strings.ToLower(issueData.Title) - // Replace spaces and special characters with dashes kebabTitle = regexp.MustCompile(`[^a-z0-9]+`).ReplaceAllString(kebabTitle, "-") - // Remove leading and trailing dashes kebabTitle = strings.Trim(kebabTitle, "-") - // Construct branch name: {issue-number}-{kebab-case-title} branchName := fmt.Sprintf("%s-%s", issueNumber, kebabTitle) - - // Ensure branch name doesn't exceed git's limit (usually around 250 chars) if len(branchName) > maximumBranchNameLength { branchName = branchName[:maximumBranchNameLength] } branchName = utils.CleanArgFromSpecSymbols(branchName) - // Add suffix "-dev" for a dev branch branchName += "-dev" - return branchName, nil + comment := IssuePRTtilePrefix + " '" + issueData.Title + "' " + body := "" + if len(issueData.Title) > 0 { + body = IssueSign + issueNumber + oneSpace + issueData.Title + } + notesObj, err := notesPkg.Serialize(issueURL, "", notesPkg.BranchTypeDev, issueData.Title) + if err != nil { + return "", nil, err + } + + return branchName, []string{comment, body, notesObj}, nil } func GetGithubIssueNameByNumber(issueNum string, parentrepo string) (string, error) { diff --git a/internal/commands/dev.go b/internal/commands/dev.go index e7af910..de6249f 100644 --- a/internal/commands/dev.go +++ b/internal/commands/dev.go @@ -10,7 +10,6 @@ import ( "github.com/atotto/clipboard" "github.com/fatih/color" - "github.com/go-git/go-git/v5/plumbing" "github.com/spf13/cobra" "github.com/untillpro/goutils/logger" "github.com/untillpro/qs/gitcmds" @@ -96,49 +95,30 @@ func Dev(cmd *cobra.Command, wd string, doDelete bool, ignoreHook bool, args []s return err } - issueNum, githubIssueURL, ok, err := argContainsGithubIssueLink(wd, args...) + issueNum, githubIssueURL, isGithubIssue, err := argContainsGithubIssueLink(wd, args...) if err != nil { return err } - checkRemoteBranchExistence := true - if ok { // github issue - fmt.Print("Dev branch for issue #" + strconv.Itoa(issueNum) + " will be created. Agree?(y/n)") - _, _ = fmt.Scanln(&response) - if response == pushYes { - branch, notes, err = gitcmds.CreateGithubLinkToIssue(wd, parentRepo, githubIssueURL, issueNum, args...) - if err != nil { - return err - } - checkRemoteBranchExistence = false // no need to check remote branch existence for issue branch - } - } else { // PK topic or Jira issue - if _, ok := jira.GetJiraTicketIDFromArgs(args...); ok { // Jira issue - branch, notes, err = jira.GetJiraBranchName(args...) - } else { - branch, notes, err = utils.GetBranchName(false, args...) - branch += "-dev" // Add suffix "-dev" for a dev branch - } - if err != nil { - // Show suggestion if issue is not found or insufficient permission to see it - // And exit silently - if errors.Is(err, jira.ErrJiraIssueNotFoundOrInsufficientPermission) { - fmt.Print(jira.NotFoundIssueOrInsufficientAccessRightSuggestion) - - return nil - } + _, isJiraIssue := jira.GetJiraTicketIDFromArgs(args...) - return err + switch { + case isGithubIssue: + branch, notes, err = gitcmds.BuildDevBranchName(githubIssueURL) + case isJiraIssue: + branch, notes, err = jira.GetJiraBranchName(args...) + default: + branch, notes, err = utils.GetBranchName(false, args...) + branch += "-dev" + } + if err != nil { + if errors.Is(err, jira.ErrJiraIssueNotFoundOrInsufficientPermission) { + fmt.Print(jira.NotFoundIssueOrInsufficientAccessRightSuggestion) + return nil } - - devMsg := strings.ReplaceAll("Dev branch '$reponame' will be created. Continue(y/n)? ", "$reponame", branch) - fmt.Print(devMsg) - _, _ = fmt.Scanln(&response) + return err } - // put branch name to command context - cmd.SetContext(context.WithValue(cmd.Context(), utils.CtxKeyDevBranchName, branch)) - exists, err := branchExists(wd, branch) if err != nil { return fmt.Errorf("error checking branch existence: %w", err) @@ -147,31 +127,38 @@ func Dev(cmd *cobra.Command, wd string, doDelete bool, ignoreHook bool, args []s return fmt.Errorf("dev branch '%s' already exists", branch) } + cmd.SetContext(context.WithValue(cmd.Context(), utils.CtxKeyDevBranchName, branch)) + + fmt.Print("Dev branch '" + branch + "' will be created. Continue(y/n)? ") + _, _ = fmt.Scanln(&response) + switch response { case pushYes: - // Remote developer branch, linked to issue is created - var response string - // Only add upstream if we have a parent repo and upstream doesn't exist - // In single remote mode (no parent repo), we don't need upstream if len(parentRepo) > 0 && !upstreamExists { + var upstreamResponse string fmt.Print("Upstream not found.\nRepository " + parentRepo + " will be added as upstream. Agree[y/n]?") - _, _ = fmt.Scanln(&response) - if response != pushYes { + _, _ = fmt.Scanln(&upstreamResponse) + if upstreamResponse != pushYes { fmt.Print(msgOkSeeYou) return nil } - response = "" if err := gitcmds.MakeUpstreamForBranch(wd, parentRepo); err != nil { return err } } - if err := gitcmds.CreateDevBranch(wd, branch, mainBranch, notes, checkRemoteBranchExistence); err != nil { + if err := gitcmds.CreateDevBranch(wd, branch, mainBranch, notes); err != nil { return err } + + if isGithubIssue { + notes, err = gitcmds.LinkBranchToGithubIssue(wd, parentRepo, githubIssueURL, issueNum, branch, args...) + if err != nil { + return err + } + } default: fmt.Print(msgOkSeeYou) - return nil } @@ -194,32 +181,24 @@ func Dev(cmd *cobra.Command, wd string, doDelete bool, ignoreHook bool, args []s return nil } -// branchExists checks if a branch with the given name already exists in the current git repository. func branchExists(wd, branchName string) (bool, error) { - repo, err := gitcmds.OpenGitRepository(wd) + cmd := exec.Command("git", "branch", "--list", branchName) + cmd.Dir = wd + output, err := cmd.Output() if err != nil { - return false, err + return false, fmt.Errorf("failed to check local branches: %w", err) + } + if strings.TrimSpace(string(output)) != "" { + return true, nil } - branches, err := repo.Branches() + cmd = exec.Command("git", "ls-remote", "--heads", "origin", branchName) + cmd.Dir = wd + output, err = cmd.Output() if err != nil { - return false, fmt.Errorf("failed to get branches: %w", err) + return false, fmt.Errorf("failed to check remote branches: %w", err) } - - // Find development branch name that starts with the issue ID - exists := false - _ = branches.ForEach(func(ref *plumbing.Reference) error { - nextBranchName := ref.Name().Short() - if nextBranchName == branchName { - exists = true - - return nil - } - - return nil - }) - - return exists, nil + return strings.TrimSpace(string(output)) != "", nil } // getArgStringFromClipboard retrieves a string from the clipboard, or uses the context value if available. diff --git a/uspecs/changes/2602231024-fix-dev-branch-creation/change.md b/uspecs/changes/2602231024-fix-dev-branch-creation/change.md new file mode 100644 index 0000000..7e592da --- /dev/null +++ b/uspecs/changes/2602231024-fix-dev-branch-creation/change.md @@ -0,0 +1,22 @@ +--- +registered_at: 2026-02-23T10:24:58Z +change_id: 2602231024-fix-dev-branch-creation +baseline: b7c87527aa6501a58538c46ba20fcb43c9962e45 +--- + +# Change request: Fix dev branch creation flow + +## Why + +The dev branch creation flow has redundant branch existence checks scattered across three separate functions (`buildDevBranchName` → `CreateGithubLinkToIssue` → `Dev()` → `CreateDevBranch`). The remote branch is checked in `CreateGithubLinkToIssue`, then local branch is checked in `Dev()`, then remote branch is checked again in `CreateDevBranch`. This makes the flow fragile, hard to follow, and inconsistent between GitHub and Jira issue workflows. + +## What + +Restructure the dev branch creation to have a single, clear pipeline for both GitHub and Jira issues: + +- Remove the branch existence check from `CreateGithubLinkToIssue` (lines 64-81 of `gitcmds/github.go`) — it checks a variable `branch` that is empty at that point (bug), and the check is redundant anyway +- Remove the `checkRemoteBranchExistence` parameter from `CreateDevBranch` and the remote existence check inside it (lines 48-64 of `gitcmds/dev.go`) — existence is already validated before this function is called +- Consolidate branch existence checking into a single place in `Dev()` (`internal/commands/dev.go`) that works uniformly for both GitHub and Jira flows +- Separate concerns: branch name resolution (with notes preparation) happens first, then a single existence check, then branch creation/push +- Unify dev branch name building in `Dev()` — currently GitHub issues use `BuildDevBranchName` while Jira/PK issues use `GetJiraBranchName`/`GetBranchName` with separate `-dev` suffix logic; should be a single path that supports all issue types +- Move `CreateGithubLinkToIssue` to happen after `CreateDevBranch` — currently the GitHub issue is linked to the branch before the branch is actually created, which is the wrong order; the branch should be created first, then linked to the issue diff --git a/uspecs/changes/2602231024-fix-dev-branch-creation/impl.md b/uspecs/changes/2602231024-fix-dev-branch-creation/impl.md new file mode 100644 index 0000000..2d14f0c --- /dev/null +++ b/uspecs/changes/2602231024-fix-dev-branch-creation/impl.md @@ -0,0 +1,41 @@ +# Implementation plan: Fix dev branch creation flow + +## Construction + +### Remove redundant branch existence checks + +- [x] update: [gitcmds/github.go](../../../gitcmds/github.go) + - remove: Remote branch existence check (lines 64-81) from `CreateGithubLinkToIssue` — it uses uninitialized `branch` variable (bug) and is redundant since `Dev()` already checks existence + +- [x] update: [gitcmds/dev.go](../../../gitcmds/dev.go) + - remove: `checkRemoteBranchExistence` parameter from `CreateDevBranch` signature + - remove: Remote branch existence check block (lines 48-64) inside `CreateDevBranch` + +### Consolidate existence check in Dev() + +- [x] update: [internal/commands/dev.go](../../../internal/commands/dev.go) + - update: Replace local-only `branchExists` check with a combined local+remote existence check that works uniformly for both GitHub and Jira flows + - update: Remove `checkRemoteBranchExistence` variable and pass updated signature to `CreateDevBranch` + - update: Move branch existence check to happen after branch name is resolved but before user confirmation prompt, so the user is not asked to confirm creation of an already-existing branch + +### Reorder GitHub branch creation and issue linking + +- [x] update: [gitcmds/github.go](../../../gitcmds/github.go) + - update: Renamed `CreateGithubLinkToIssue` to `LinkBranchToGithubIssue`, changed signature to return `(notes []string, err error)` instead of `(branch string, notes []string, err error)` since branch name is already known + +- [x] update: [internal/commands/dev.go](../../../internal/commands/dev.go) + - update: Reordered calls so `CreateDevBranch` runs first (creates local branch + pushes to origin), then `LinkBranchToGithubIssue` links the existing remote branch to the GitHub issue + +### Unify dev branch name building + +- [x] update: [gitcmds/github.go](../../../gitcmds/github.go) and [internal/commands/dev.go](../../../internal/commands/dev.go) + - update: Changed `BuildDevBranchName` signature from `(string, error)` to `(string, []string, error)` — now returns notes (old-style comment, body, and JSON notes) alongside the branch name, matching the Jira/PK pattern + - update: Removed note preparation from `LinkBranchToGithubIssue` (now returns `nil, nil` — linking only) + - update: Unified `Dev()` branch resolution into a single `switch` dispatching to `BuildDevBranchName` / `GetJiraBranchName` / `GetBranchName`, all producing `(branch, notes, error)` + +### Tests and review + +- [ ] review + - `go build ./...` compiles without errors + - `go vet ./...` passes +