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

hub checkout pulls down tags from user's fork #879

Closed
dbingham opened this issue Apr 23, 2015 · 10 comments · Fixed by #905
Closed

hub checkout pulls down tags from user's fork #879

dbingham opened this issue Apr 23, 2015 · 10 comments · Fixed by #905

Comments

@dbingham
Copy link

If the user has tags in the history on their fork, doing a hub checkout <pull req URL> will also pull down their tags. Would it be possible to add the --no-tags option to the fetch call to prevent that?

@mislav
Copy link
Owner

mislav commented Apr 27, 2015

We're going to switch to a strategy where we will not even add or fetch a user's fork when checking out a PR in the future. Stay tuned.

@dbingham
Copy link
Author

@mislav Any updates on when that switch will happen? And will I still be able to make changes to the branch and push it? i.e. rebase the changes and push it so that the PR triggers a new CI build?

@mislav
Copy link
Owner

mislav commented May 16, 2015

No promises when the switch will happen exactly, but here is my rough plan:

  • If the head of the PR is in the same repo that you already have a remote set up for (e.g. "origin" repo), hub checkout <PULLREQ-URL> will check out the latest state of the "origin/feature" branch. You should be able to push to the same branch with git push to update it and re-trigger CI.
  • If the PR is from a fork you don't have a remote set up for (and probably you don't have write access to that fork either), instead of setting up a new git remote and pulling from that, I want hub to check out refs/pull/42/head from the "origin" remote which is the latest state of PR #42. You will get the latest code but you won't be able to push to the same branch as the PR. However, with git push origin HEAD:<feature> you can manually push the branch to any new branch if you want to trigger CI, and with hub ci-status you can check if the CI check passed.

@dbingham
Copy link
Author

@mislav Here's my scenario:

We're on GitHub Enterprise, and have an organization with private repos. We grant read-only access to developers who then fork the repo and submit pull requests. In that scenario, GitHub seems to copy the ACLs from the upstream repo. Since I (and the others who process PRs) do have write access to upstream, we also get write access to the forks.

Further, it is often the case that a developer submits a pull request and then doesn't respond to requests for minor tweaks. Or they go on vacation, or I'm working at night. Lots of reasons, but there are real scenarios where I want to checkout the PR (as a convenience), make a tweak (squash some commits, rebase, whatever), and then push the changes back to their fork.

The reason I need to push back to the branch on their fork is because it triggers a new CI build (with the GitHub integration). Our development process requires that CI builds/passes tests before the PR can be merged.

In any case, the tool works great for what I want right now. I just want to add --no-tags so that I don't accidentally pull down other people's tags.

Hope that makes sense.

@mislav
Copy link
Owner

mislav commented May 17, 2015

Thanks, that makes sense. But, it doesn't seem that this workflow might be widespread. People don't usually do work and push it to other people's forks, especially when they didn't have that fork set up as a git remote in the first place.

I might not cover the scenario you're describing. Maybe a workaround for you could be that you manually say git remote add -f --no-tags <fork-username>?

@dbingham
Copy link
Author

I can tweak my scripts to run whatever git commands I want. That wasn't really the point. I can look at the history without hub. Its right there on the web. The value in hub for me was the ability to quickly and easily fix pull requests that were broken. In particular, it was valuable to be able to then push that fix back up to a place where github would notice and re-invoke the CI builds (not something I can do based on my local clone).

In any case, all I was hoping for was a minor tweak to the command that you run in hub. Obviously, if I spend a few minutes learning go I can fork the project and do whatever I want. But I suspect that you could satisfy all users if you just changed the default behavior to do whatever you think is best, but have an option that falls back to the old behavior (since you've already released that and presumably some users depend on that behavior). If you were open to that, I'd just ask again what I asked before: Can you tweak it so that it adds the --no-tags flag to the fetch? If you're not open to that, just close this issue and I'll write my own scripts and stop recommending hub to my team.

@mislav
Copy link
Owner

mislav commented May 21, 2015

Can you tweak it so that it adds the --no-tags flag to the fetch? If you're not open to that, just close this issue and I'll write my own scripts and stop recommending hub to my team.

That sounds a bit like blackmail.

But I get what you're saying. You don't want to wait around for the next bigger release of hub where we change the internals of some commands if that's not going to help your cause. You're asking can we do a quick fix right now. I've proposed the change in #905, so please take a look.

@dbingham
Copy link
Author

Ha! I can see that, though not what I meant. I just assume that me being an active user (or not) makes no difference to your life, so if it were blackmail it wouldn't have much in the way of teeth.

@dbingham dbingham reopened this May 21, 2015
@dbingham
Copy link
Author

Clicked the wrong button...

(cont)
All I meant was that when you do change your approach, seems to me that you could support both styles. hub checkout --with-fork... would do the current way, but default would do the new way you described. Then both use cases could be easily supported. But, if that doesn't sound like something you want to do, I can understand that. I just won't have a use case for hub any more as this capability is why I use it. No threat implied there, I can come up with other solutions. I just mention it as a data point for your consideration (I.e. If one user depends on this, perhaps >1 user depends on this, etc).

@dbingham
Copy link
Author

#905 looks reasonable to me.

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 a pull request may close this issue.

2 participants