Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

pr_forget() #1263

Closed
hadley opened this issue Oct 29, 2020 · 6 comments
Closed

pr_forget() #1263

hadley opened this issue Oct 29, 2020 · 6 comments
Labels
git git, GitHub, and CI in general

Comments

@hadley
Copy link
Member

hadley commented Oct 29, 2020

We need some function to terminate a pr (without merging it) if you decide that fetching or creating it was a bad idea. Currently, I do git checkout master then git branch -D <branch>, but that doesn't remove any remotes that might've been added. I don't think this would touch remote branches.

@hadley hadley changed the title pr_abort() pr_forget() Oct 29, 2020
@jennybc jennybc added the git git, GitHub, and CI in general label Oct 30, 2020
@jennybc jennybc added this to the v2.0.0 milestone Oct 30, 2020
@jtr13
Copy link

jtr13 commented Nov 16, 2020

Yes, this would be super helpful. My use case is examining student PRs and telling them what to fix. What I'm doing now (with help from ROpenSci slack) while on the PR branch to jump ship is:

$ git reset --hard [<commit>] (include PR branch <commit> if I've made commits)
$ git clean [-d] -f (include -d to delete new subdirectories)

> pr_finish()

@jennybc
Copy link
Member

jennybc commented Nov 16, 2020

I think it's fair to say that what we want from pr_forget() is to return one's local Git state to where it was before we did pr_fetch(), yeah?

@hadley
Copy link
Member Author

hadley commented Nov 16, 2020

Yeah, exactly.

@jennybc
Copy link
Member

jennybc commented Nov 19, 2020

Note to self: also handle the case where the whole thing started with pr_init()

@jennybc
Copy link
Member

jennybc commented Nov 23, 2020

@hadley Are these all the ways in which pr_forget() is not just pr_finish()? I've updated pr_finish() and I want to understand how they differ.

For pr_forget():

  • The target branch need not have been pushed and made into a PR (pr_finish() stops if there's no associated PR)
  • It always involves a local branch, i.e. it's never a remote-only operation (pr_finish() can now delete the remote branch associated with a merged PR, even if we never fetched it)
  • We never delete a remote branch (pr_finish() will do so, under certain circumstances)
  • We do not fret about commits that only exist locally (pr_finish() informs the user about such commits and asks if they want to proceed)

@hadley
Copy link
Member Author

hadley commented Nov 23, 2020

I was just thinking about the last bullet this weekend — I think it probably needs to warn that you have work that won't be pushed, and require confirmation to continue. Otherwise, that sounds correct to me.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
git git, GitHub, and CI in general
Projects
None yet
Development

No branches or pull requests

3 participants