Skip to content
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

pr checkout fails if the author had force-pushed commits #2061

Open
kumar303 opened this issue Mar 1, 2019 · 6 comments
Open

pr checkout fails if the author had force-pushed commits #2061

kumar303 opened this issue Mar 1, 2019 · 6 comments
Labels

Comments

@kumar303
Copy link

kumar303 commented Mar 1, 2019

Consider this work flow:

  • Janice asks me to review her pull request
  • I run hub pr checkout 123
  • I request changes; Janice rebases on master and force-pushes to her branch while making changes
  • I run hub pr checkout 123 again

At this point, since I already had the branch checked out, hub tries to fast forward my branch. This fails because there are conflicts.

I would love to have a flag like --force or --recreate-branch that deletes and recreates the branch for me. Since I am only reviewing the patch, there is no harm in deleting the branch.

$ hub version
git version 2.14.1
hub version 2.10.0
@kumar303
Copy link
Author

kumar303 commented Mar 8, 2019

One idea here is to introduce a new command: hub pr tmp-checkout. This would always create a branch with a temp prefix (or some other identifier) so that hub knows that it's safe to delete and recreate the branch.

@mislav
Copy link
Owner

mislav commented Mar 12, 2019

Thanks for writing in! I'm leaning to having hub pr checkout always reset the branch to the current state of PR head branch, regardless of whether it was a fast-forward or not. That would enable you to practice this workflow without passing any extra arguments. However, if you yourself had added local commits to the branch and then checked out Janice's work, your local commits would be "lost". Could this be a potential issue?

@mislav mislav added the feature label Mar 12, 2019
@diegosueiro
Copy link

My 2 cents.
I think that should be implemented the choice for replacing or not the old copy of the PR.
The reason for this is that sometimes I'm interested in checking only the differences that were force pushed and maintaining the old copy allows me to do it.

@kumar303
Copy link
Author

kumar303 commented Jun 3, 2019

However, if you yourself had added local commits to the branch and then checked out Janice's work, your local commits would be "lost". Could this be a potential issue?

Yeah, the commits would be lost (or very hard to find). It would be surprising if something like hub pr checkout ... could destroy existing history. I'd be less surprised if something named hub pr tmp-checkout destroyed history.

@kumar303
Copy link
Author

kumar303 commented Jul 3, 2019

I've been using hub pr checkout... for my own branches a lot lately. It's easier than typing the branch name 😄 Because of this, I think it would be better to introduce a new command such as tmp-checkout to implement a truly temporary, self-destructable checkout feature.

@paride
Copy link

paride commented Jul 21, 2020

A force flag as in hub pr checkout --force would make the overwrite explicit without introducing a new command, and mirrors the git push --force command that has been used to update the branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants