-
Notifications
You must be signed in to change notification settings - Fork 40
feat (cleanup forks): detect deletable forks #168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Agreed to leave the |
f4d5a90 to
9e49f56
Compare
There was a problem hiding this 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 implements functionality to detect and clean up deletable forks in Turbolift campaigns. It adds a new cleanup command that identifies forks without open upstream PRs and provides instructions for their safe deletion.
- Adds new GitHub API methods to check if a repository is a fork and detect open upstream PRs
- Implements Git functionality to retrieve origin repository URLs
- Creates a new cleanup command with comprehensive test coverage
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/github/github.go | Adds IsFork, UserHasOpenUpstreamPRs, and GetOriginRepoName methods to GitHub interface |
| internal/github/github_test.go | Comprehensive test coverage for new GitHub methods |
| internal/github/fake_github.go | Mock implementations for testing the new GitHub methods |
| internal/git/git.go | Adds GetOriginUrl method to retrieve Git origin URL |
| internal/git/git_test.go | Test coverage for the new GetOriginUrl method |
| internal/git/fake_git.go | Mock implementation for GetOriginUrl method |
| internal/executor/fake_executor.go | New fake executors that return boolean values for testing |
| cmd/cleanup/cleanup.go | New cleanup command implementation |
| cmd/cleanup/cleanup_test.go | Test suite for the cleanup command |
| cmd/root.go | Registers the new cleanup command |
| README.md | Documentation for the cleanup functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| func (r *RealGitHub) IsFork(output io.Writer, workingDir string) (bool, error) { | ||
| response, err := execInstance.ExecuteAndCapture(output, workingDir, "gh", "repo", "view", "--json", "isFork") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly I think that gh repo view --json isFork is not right. When running it locally I'm seeing it not detect forks...
Looking deeper, I think that gh repo view --json isFork is telling us about the cloned repo, not the user's fork of it. e.g. :
$ cd work/someorg/somerepo
$ gh repo view --json isFork,nameWithOwner
{
"isFork": false,
"nameWithOwner": "someorg/somerepo"
}
Note that git remote -v yields:
origin [email protected]:richardnorth/somerepo.git (fetch)
origin [email protected]:richardnorth/somerepo.git (push)
upstream [email protected]:someorg/somerepo.git (fetch)
upstream [email protected]:someorg/somerepo.git (push)
I've been battling with permissions on GHES but I think that if we were to use the name of the origin repo we'd be able to get the right answer. e.g. gh repo view richardnorth/somerepo --json isFork
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch - I'll update that. Should be straightforward since we're already adding a method that gets the origin repo name.
for #6 and #167