Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Nov 19, 2025

Summary

  • Introduced a new --force flag for the PR transfer command
  • Enables transferring PRs with conflicts by creating a PR with conflict markers
  • Provides more flexibility when handling complex patch applications

Changes

  • Updated transferPR and applyPatchToRepo functions to support --force flag
  • Added command-line flag and updated help text
  • Implemented conflict handling mechanism with optional force mode
  • Updated tests to validate new flag existence

Behavior

  • Without --force: Fail on conflicts and provide an error message
  • With --force:
    • Create a PR with conflict markers and .rej files
    • Allow manual conflict resolution
    • Include warning in commit message about existing conflicts

Copilot AI review requested due to automatic review settings November 19, 2025 08:11
Copilot finished reviewing on behalf of dsyme November 19, 2025 08:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a --force flag to the PR transfer command that enables transferring PRs with conflicts by creating a PR with conflict markers and .rej files, providing more flexibility when handling complex patch applications that don't apply cleanly.

  • Added --force/-f flag to PR transfer command
  • Modified conflict handling to create PRs with conflict markers when --force is enabled
  • Enhanced commit messages to warn about included conflicts
  • Updated help text with force flag usage examples

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
pkg/cli/pr_command.go Added --force flag parameter throughout transfer flow, implemented conflict handling logic with --reject option, and enhanced commit messages to indicate presence of conflicts
pkg/cli/pr_command_test.go Added test to verify --force flag exists on transfer subcommand

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

if verbose {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Applying patch with --reject to create conflict markers..."))
}
rejectCmd := exec.Command("git", "apply", "--reject", patchFile)
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rejectCmd variable is declared twice in the same code path. It's first declared on line 376 (inside the verbose block) and then redeclared on line 393 (in the force mode block). While this works due to Go's scoping rules, it could lead to confusion.

Consider using a single declaration or using different variable names to make the code clearer.

Copilot uses AI. Check for mistakes.
Comment on lines +408 to +411
rejFilesCmd := exec.Command("sh", "-c", "find . -name '*.rej' -type f")
rejFilesOutput, _ := rejFilesCmd.Output()
hasConflicts := len(strings.TrimSpace(string(rejFilesOutput))) > 0

Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using sh -c "find . -name '*.rej' -type f" to detect .rej files may have portability issues on Windows systems where sh might not be available. Additionally, running find from the current directory (.) could potentially search the entire repository including unrelated directories.

Consider using Go's filepath.Walk or filepath.Glob for better portability, or at least constraining the search to the git working tree with git ls-files '*.rej'.

Suggested change
rejFilesCmd := exec.Command("sh", "-c", "find . -name '*.rej' -type f")
rejFilesOutput, _ := rejFilesCmd.Output()
hasConflicts := len(strings.TrimSpace(string(rejFilesOutput))) > 0
hasConflicts := false
err := filepath.WalkDir(".", func(path string, d os.DirEntry, err error) error {
if err != nil {
return err
}
if !d.IsDir() && strings.HasSuffix(d.Name(), ".rej") {
hasConflicts = true
// Stop walking as soon as we find one
return filepath.SkipDir
}
return nil
})
if err != nil && verbose {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Error searching for .rej files: %v", err)))
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants