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
Fetch upstream remote after adding it #752
Conversation
@@ -143,7 +143,7 @@ func addUpstreamRemote(parentRepo ghrepo.Interface, cloneURL string) error { | |||
upstreamURL := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(parentRepo)) | |||
cloneDir := path.Base(strings.TrimSuffix(cloneURL, ".git")) | |||
|
|||
cloneCmd := git.GitCommand("-C", cloneDir, "remote", "add", "upstream", upstreamURL) | |||
cloneCmd := git.GitCommand("-C", cloneDir, "remote", "add", "-f", "upstream", upstreamURL) |
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.
I was wondering if it might be possible – and preferred – to instead repurpose and use an adjusted version of AddRemote
:
Lines 74 to 76 in e10ccef
// AddRemote adds a new git remote and auto-fetches objects from it | |
func AddRemote(name, u string) (*Remote, error) { | |
addCmd := exec.Command("git", "remote", "add", "-f", name, u) |
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.
That is generally a good idea, but in this instance it would require us to change the signature of AddRemote to accept an additional (optional) directory parameter.
Since making that change isn't a marked improvement over your current approach, in my book, I would just say— this looks good, and leave it as is! Thank you so much for taking a stab at a fix 🎉
@@ -143,7 +143,7 @@ func addUpstreamRemote(parentRepo ghrepo.Interface, cloneURL string) error { | |||
upstreamURL := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(parentRepo)) | |||
cloneDir := path.Base(strings.TrimSuffix(cloneURL, ".git")) | |||
|
|||
cloneCmd := git.GitCommand("-C", cloneDir, "remote", "add", "upstream", upstreamURL) | |||
cloneCmd := git.GitCommand("-C", cloneDir, "remote", "add", "-f", "upstream", upstreamURL) |
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.
That is generally a good idea, but in this instance it would require us to change the signature of AddRemote to accept an additional (optional) directory parameter.
Since making that change isn't a marked improvement over your current approach, in my book, I would just say— this looks good, and leave it as is! Thank you so much for taking a stab at a fix 🎉
This is a naive proposal for fixing #731