Permalink
Browse files

Deprecate converting issue to pull-request

This feature is likely to get dropped from GitHub API in the near
future. The alternative is to simply create a new pull-request and
reference the original issue in the description.
  • Loading branch information...
1 parent 5e130ac commit 4f70dd126f46dec14fc341c97c18efae417743c7 @mislav mislav committed Dec 21, 2013
Showing with 21 additions and 19 deletions.
  1. +0 −3 README.md
  2. +10 −2 features/pull_request.feature
  3. +4 −1 lib/hub/commands.rb
  4. +1 −4 man/hub.1
  5. +3 −6 man/hub.1.html
  6. +3 −3 man/hub.1.ronn
View
@@ -232,9 +232,6 @@ superpowers:
# explicit title, pull base & head:
$ git pull-request -m "Implemented feature X" -b defunkt:master -h mislav:feature
- $ git pull-request -i 123
- [ attached pull request to issue #123 ]
-
### git checkout
$ git checkout https://github.com/defunkt/hub/pull/73
@@ -342,7 +342,11 @@ Feature: hub pull-request
}
"""
When I successfully run `hub pull-request -i 92`
- Then the output should contain exactly "https://github.com/mislav/coral/pull/92\n"
+ Then the output should contain exactly:
+ """
+ https://github.com/mislav/coral/pull/92
+ Warning: Issue to pull request conversion is deprecated and might not work in the future.\n
+ """
Scenario: Convert issue URL to pull request
Given I am on the "feature" branch with upstream "origin/feature"
@@ -354,7 +358,11 @@ Feature: hub pull-request
}
"""
When I successfully run `hub pull-request https://github.com/mislav/coral/issues/92`
- Then the output should contain exactly "https://github.com/mislav/coral/pull/92\n"
+ Then the output should contain exactly:
+ """
+ https://github.com/mislav/coral/pull/92
+ Warning: Issue to pull request conversion is deprecated and might not work in the future.\n
+ """
Scenario: Enterprise host
Given the "origin" remote has url "git@git.my.org:mislav/coral.git"
View
@@ -115,7 +115,6 @@ def ci_status(args)
# $ hub pull-request
# $ hub pull-request "My humble contribution"
- # $ hub pull-request -i 92
# $ hub pull-request https://github.com/rtomayko/tilt/issues/92
def pull_request(args)
args.shift
@@ -173,6 +172,10 @@ def pull_request(args)
end
end
+ if options[:issue]
+ warn "Warning: Issue to pull request conversion is deprecated and might not work in the future."
+ end
+
options[:project] = base_project
options[:base] ||= master_branch.short_name
View
@@ -152,7 +152,7 @@ Opens a pull request on GitHub for the project that the "origin" remote points t
Without \fIMESSAGE\fR or \fIFILE\fR, a text editor will open in which title and body of the pull request can be entered in the same manner as git commit message\. Pull request message can also be passed via stdin with \fB\-F \-\fR\.
.
.IP
-If instead of normal \fITITLE\fR an issue number is given with \fB\-i\fR, the pull request will be attached to an existing GitHub issue\. Alternatively, instead of title you can paste a full URL to an issue on GitHub\.
+Issue to pull request conversion via \fB\-i <ISSUE>\fR or \fIISSUE\-URL\fR arguments is deprecated and will likely be removed from the future versions of both hub and GitHub API\.
.
.TP
\fBgit ci\-status\fR [\fB\-v\fR] [\fICOMMIT\fR]
@@ -313,9 +313,6 @@ $ git pull\-request
# explicit title, pull base & head:
$ git pull\-request \-m "Implemented feature X" \-b defunkt:master \-h mislav:feature
-
-$ git pull\-request \-i 123
-[ attached pull request to issue #123 ]
.
.fi
.
View
@@ -192,9 +192,9 @@ <h2 id="DESCRIPTION">DESCRIPTION</h2>
of the pull request can be entered in the same manner as git commit message.
Pull request message can also be passed via stdin with <code>-F -</code>.</p>
-<p>If instead of normal <var>TITLE</var> an issue number is given with <code>-i</code>, the pull
-request will be attached to an existing GitHub issue. Alternatively, instead
-of title you can paste a full URL to an issue on GitHub.</p></dd>
+<p>Issue to pull request conversion via <code>-i &lt;ISSUE></code> or <var>ISSUE-URL</var>
+arguments is deprecated and will likely be removed from the future versions
+of both hub and GitHub API.</p></dd>
<dt><code>git ci-status</code> [<code>-v</code>] [<var>COMMIT</var>]</dt><dd><p>Looks up the SHA for <var>COMMIT</var> in GitHub Status API and displays the latest
status. Exits with one of:<br />
success (0), error (1), failure (1), pending (2), no status (3)</p>
@@ -319,9 +319,6 @@ <h3 id="git-pull-request">git pull-request</h3>
# explicit title, pull base &amp; head:
$ git pull-request -m "Implemented feature X" -b defunkt:master -h mislav:feature
-
-$ git pull-request -i 123
-[ attached pull request to issue #123 ]
</code></pre>
<h3 id="git-checkout">git checkout</h3>
View
@@ -151,9 +151,9 @@ hub also adds some custom commands that are otherwise not present in git:
of the pull request can be entered in the same manner as git commit message.
Pull request message can also be passed via stdin with `-F -`.
- If instead of normal <TITLE> an issue number is given with `-i`, the pull
- request will be attached to an existing GitHub issue. Alternatively, instead
- of title you can paste a full URL to an issue on GitHub.
+ Issue to pull request conversion via `-i <ISSUE>` or <ISSUE-URL>
+ arguments is deprecated and will likely be removed from the future versions
+ of both hub and GitHub API.
* `git ci-status` [`-v`] [<COMMIT>]:
Looks up the SHA for <COMMIT> in GitHub Status API and displays the latest

45 comments on commit 4f70dd1

@jricketson

It would be annoying if this was removed from the API.

@marijn

Why? This is such a nice feature.

@mislav
Member

I've explained in the commit message: the feature might get dropped from GitHub API in the future. I wrongly said "in the near future" when it fact it can only get dropped from API v4, because API v3 (current) needs to stay frozen.

The feature isn't working always and a lot of people are confused when they get validation error messages back from GitHub. Sometimes they're trying to transform the issue which is not theirs; sometimes GitHub just decides it won't let them transform an issue even if the user is an admin of the repo.

Because the user experience around this feature isn't great, and a lot of people don't know precisely how it works, I decided I don't want to support it in hub anymore.

@marijn
@rsanheim

Because the user experience around this feature isn't great, and a lot of people don't know precisely how it works, I decided I don't want to support it in hub anymore.

Just to expand a bit on this, over the many years I've used GitHub, and the almost two years I've worked at GitHub, I've seen over and over again that it is better to reference related items (i.e. other issues or pulls) rather than do any sort of transformation on them.

Transformation is a destructive act and is often confusing to participants. References are simple, leave the underlying issues in place as is, and are easier and simpler to understand in the long run. References also allow more flexibility, for example where one underlying issue spawns multiple, more focused pull requests that then refer back to the original issue.

So yeah, please do look to references over transformation, as I would expect the issue -> pull transformation to go away eventually. 😃

@hkdobrev

Transformation is a destructive act and is often confusing to participants.

+1

@sirkitree

This is disappointing. We use this feature all the time. The idea of attaching code to an issue is what I really identify with. I could care less about the transformation of an issue to a pull request, it's just semantics to me really, but attaching code to an issue that fixes that issue is totally valuable IMHO.

@davereid

Being able to convert an issue to a pull request is one the of major saving graces of working in the Github issue queues for myself and a lot of people that I know. I hope this API will remain, otherwise I will probably be looking at alternatives to Github with better issue queue systems.

@marijn

Transformation is a destructive act [...]

I don't really understand that either. It's not destructive. No information gets lost. How should I read that?

@unn
unn replied Jan 17, 2014

-1

@rsanheim

I don't really understand that either. It's not destructive. No information gets lost. How should I read that?

Okay, I guess I'll dive in here. 😃 All of the below is from extensive use of Issues and Pulls, not only on GitHub.com (approaching 20k issues right now...) but also on many open source projects, large and small.

Imagine I create an issue on your open source project. The issue is a bug report, lets call it "API doesn't handle deletes properly". I provide an okay bug report but it needs more details because others can't reproduce. We discuss back and forth for awhile, there are comments from other people and maybe some references to similar or related bug reports. Three weeks later, you come up with a possible fix and use transform to change my bug report into a pull request.

In the best case scenario, the fix is good and the pull gets merged. But now the title of that pull request doesn't make sense - a sensible title would is now "Fix API to delete properly for 2.0 clients (like rsanheim)", or whatever, something specific about the fix. Much of the original discussion around the bug report doesn't make sense, either, so the history on that issue looks weird. Yes, you could go edit the title and maybe even edit the body of the issue, but now you are really mucking with data and rewriting history.

In a less happy scenario, you do the transform, and the pull request is controversial. Now, the discussion in that thread turns from a bug report into a discussion of implementation. Maybe there is a lot of low level code review and bickering going back forth. The original bug and the question of "is this really a problem?" is now lost.

In a really crappy scenario, you do the transform, iterate on it a bit, and realize your fix won't work. You went down a bad path and want to do a different fix, or there are 3 other fixes that need to happen before you can fix the actual issue. You close the pull request (even though the original issue is still a valid issue), and now open up new pull requests, and clumsily refer back to the original, closed, pull request. Or maybe you open up a new issue and copy over some of the relevant data from the original issue.

Okay, given all those scenarios, instead imagine just using newly opened pull requests that refer back to the original issue. You can discuss implementation (on pulls) and the issue (on the issue) itself till you are blue in the face in separate threads. You can now debate different possible fixes to the same solution by opening up the different pull requests, and you still keep the original issue context as an issue. You can now review possible fixes, wait awhile, and then decide that the fix is worse then the bug, and close the issue as "wontfix" with a nice, polite summary at the end.

I hope that helps explain some of the thought process behind this. References are free, and pull requests are cheap, so don't be afraid to create a ton of them and leave your history out there for context and future developers to learn from.

@marijn
@marijn

If you log out you can see that @github is still marketing Pull Requests as Issues + Code + Comments.

@hkdobrev

@madrid

If you log out you can see that @github is still marketing Pull Requests as Issues + Code + Comments.

Of course it is, but it is not related to converting issues to pull requests.

To summarize @rsanheim reply, not only conversion creates confusion, but GitHub is replacing it with issue referencing and auto-closing (via commits and pull requets).

You could attach multiple pull requests to an issue and whichever gets merged would close the issue. This workflow works for both open-source and closed-source projects, for both people who know everything about the GitHub API and those who only read what's on the issue/pull page.

Attaching code to an issue was introduced 3.5 years ago. It never really took off. Most people don't know about it.

Referencing issues from commits and pull requests is just a better workflow and it is better on so many fronts.

@mislav
Member

Thanks for your insight, @rsanheim.

To reiterate on what I already stressed out earlier: GitHub API v3 will continue to support conversion because we're not breaking existing APIs. I deprecated the feature in hub not just because API v4 might drop it, but because I wanted to discourage people from using the feature.

@wjwwood

-1, this makes my workflow worse:

  • Make an issue to track feature
  • Convert the issue to a pull request when an implementation exists
  • Wait for reviews
  • Wait for CI
  • Merge

I understand the position behind referencing over conversion, but I believe the core problem is making pulls a degenerate and extended form of issues. I would rather see issues and pulls have clearer semantics, where issues have 0 to many proposed pulls and the pulls themselves are not issues. This would allow you to attach pull requests to issues after their creation, which I know is not much different than referencing, but it does create less overhead in terms of links I have to click and issues I have to create. It also makes it more concise for people getting up to speed on existing issues.

There are other places where the current semantics of pull requests cause weirdness in the user interface, for instance, you cannot add labels to pulls from the pull page, but you can from the issue list and the API. I would guess these "features" of pull requests are on the chopping block too. If you are going to take these away, then you need to reconsider the relationship between pull requests and issues.

IMO, if you make it such that issues are not convertible into pull requests then you should not make pull requests and issues so similar.

@davereid

@hkdobrev:

You could attach multiple pull requests to an issue and whichever gets merged would close the issue.

This doesn't work because if I make a new pull request, an issue is opened for me, when I only wanted to reference it from an existing issue and not create yet another duplicate issue. Indeed I agree with @wjwwood that the basic problem is that pull request is a subclass of issue.

@hkdobrev

@davereid You have taken my quote a bit out of context. I'm sorry for not being very clear. I meant you could reference an issue from multiple pull requests. You could use the PR description and/or the commit messages in the PR with fix/fixes/close/closes/resolve/resolves #issue-number. The repo maintainer (for example) could choose one of the pull requests and whichever gets merged would close the issue.

@scribu

Completely agree with what @wjwwood said above.

@anthonyalberto

+1 for @wjwwood comment.

This is actually one of the best features to keep discussion regarding an issue in the same place. Maybe some don't like transformation, but having two places (issue + PR) to discuss the exact same problem is a lot more confusing I find. In the end, why not let people decide which workflow they prefer?

@kyanny

+1 for removing transforming feature from hub.
I used transforming in past, but long-running issue/PR won't be fixed only one issue/PR - but if PR is merged, it can't reopen even if it's not only PR but actually issue.
User have to create another issue(s) for tracking same issue, so eventually more than one issues + PRs are needed.

@justinmayer

Please don't remove this very useful feature. Just because some folks have trouble with it doesn't mean it isn't useful for the vast majority.

@robmuh

What @rsanheim said +1 from a github noob but long-time L3 engineer--especially agree with keeping both and having really good linking. Consider what happens when an issue needs to be reopened later, a very common occurrence (at least in IBM L3 land, and yeah, don't even ask me about the bloated crap we have to use, github is like pure oxygen in comparison even if transformation stays, great job github guys, who do I send beer money too?).

@mattr-

👍 to removing it. I used to use this a lot, but then realized that references gave me more flexibility so I don't use it anymore.

@cmer
cmer replied Jan 24, 2014

👎 👎 👎 👎 👎 👎 Please don't remove this. This is one of the most useful features of Github. Literally every issue we have gets converted into a pull request. It's the best way to keep clutter to a minimum and stay organized. Please.

@patcon

Seems people will decide to stop using the transformation feature per-project when they meet workflow complexity. Why not let them decide on their own when they want to change behaviour? Is the API method really that bothersome that a best-practice for some must be forced on all... for the sake of slimming down the API by one method? (These are rhetorical questions btw.)

@mislav
Member

You can still use this feature from hub. You'll just get the deprecation warning on stderr. GitHub API v3 will continue to support this feature as well, and hub will likely stay on v3 even after v4 comes out (whenever that may be).

@patcon

👍

@cbeams

I'm strongly in favor of preserving this functionality as well. See my comments on this issue in #532.

With that said, @mislav, I understand the decision to deprecate this feature in hub if the writing is on the wall that API v4 will remove transformation altogether. So that's not really at issue here. Is there a better place where we can speak directly to the folks that are responsible for those changes in v4? Perhaps there are motivations we don't know about; perhaps there is a better and more comprehensive workflow for pull requests on the horizon (perhaps something like @wjwwood suggests above). In any case, it should be clear from this discussion that there are plenty of folks who rely on this workflow every day. I'd like to know that the folks responsible for the API are at least aware of this fact, and ideally to hear their rationale for removing it anyway (if indeed they do).

Thanks.

@raelgc

@rsanheim, @mislav I agree with the worst case scenario. But for simple and small projects, PR on issue works fine. Why don't keep both as today? Reference and PR on issue? You're saying is better use references in complex cases, and I agree, but I don't understand why remove the existing feature, which works fine, in some future release. Why don't keep providing the power of choice?

@jfelchner

I just got to this thread. This is some awesome discussion. I mean at least no one said "Pull Requests are dead" 😄

@rsanheim and @mislav I know that this doesn't directly relate to hub but since you all both work at Github (and obviously are passionate about this particular issue) I just wanted to throw some feedback into the mix.

What I Agree On

I can certainly see on open source projects (or even at @github) where converting issues to PRs would make sense. If you have multiple PRs coming in for a specific issue and are picking the best one, etc, etc, just as @rsanheim explained above. I completely get it and agree that for those situations it makes sense.

What I Disagree On

However, for closed source projects and for smaller teams, this is rarely ever the case. You almost exclusively have one set of code attached to one issue. Done. By having "code conversation in the PR" and "issue conversation in the issue" all you're doing is making it more difficult to see the entire conversation around a feature or bug. Again, for a large feature, open source, this may be desired, but for small stuff, it's just extra noise.

If it's the case that later on, something having to do with the issue (and the code attached) is buggy 🐛, then you reference the new issue (which is a bug) with the old issue (the feature). That bug then gets turned into a PR with code attached and the original issue has a reference to that bug.

Problems with References

The problem with using references is that they go everywhere and eventually you end up with spaghetti. You can't control who makes references so you'll have references on the PR going back to the issue, and references on the issue going to another PR, and references on that PR going back the original issue. Do you want this sometimes? Sure. It's nice to have that flexibility, but when you're trying to bring someone on and tell them to follow the story regarding everything related to an issue, it's a pain in the butt.

Possible "Middle Ground" Solution

Now, all that to say that I don't think that this would be a necessarily bad step if:

  • You could link one or more PRs to an issue (not a reference). There needs to be a distinction between "this PR has code that affects this issue" and "hey Jane, maybe this is related to #415" Oh, did I accidentally just make a reference to an issue that I now can't remove? See my previous point about reference spaghetti.
  • Users had the option to not allow comments on PRs so that the admin could be certain that all conversations were kept in the issue (that may not be everyone's workflow, but it's certainly a lot of people's).
  • An issue with a single PR had a visually optimized layout when viewing it
  • An issue with multiple PRs had some sort of easier way to see the PRs linked to it rather than just "scroll down through the issues and try to find them"
  • PRs stop being treated at though they are issues
  • (And a nice to have from my Christmas wishlist) a web graph showing all the references between issues and PRs (not completely related, but I thought I'd throw it into this novel of a comment. 😄
@MikeA-

Another strong! vote to keep this in the API until such date that you can click a button in the github UI to convert an existing issue into a pull request.

Having discussion start in an issue, then continue in the separate PR later when it is created through the UI, is hard to manage and although you could agree to keep code comments in the PR and other discussion in the issue or something like that, that is unenforceable. Much nicer that the issue can be the actual PR.

@pglombardo

I'm a big fan of hub - thanks for all the work! Unfortunately this feature is the #1 reason I use hub and is a core part of the workflow on our team.

The ability to convert an issue into a PR gives an very nice end-to-end lifecycle view for that issue (or enhancement) instead of having to click through to multiple places.

For a converted issue, the current one page view of the issue, discussion, commits, peer evaluation, Travis test results and eventual merge is excellent.

Hopefully you'll reconsider for hub and the Github API won't drop the ability in v4. 😉

@sun
sun replied Jul 24, 2014

Technical details aside, GH issues and PRs are suffering from a split of communication streams.

One discussion on-issue (possibly more), one per PR, and communication too easily gets out of control. Previously communicated details are lost in closed PRs, nirvana.

I never used the feature in question, but not being able to convert issues into PRs seems like a step in the right direction to me:

Centralizing communication and collaboration on the issue. All discussion on 0-N PRs should circle back into the issue. Removing the ability from PRs to contain any content (aside from code) would be positive. In turn, removing the ability to convert issues into PRs sounds like a necessary intermediate step to achieve that goal.

@meesterdude

is this going to go anywhere? it seems GH needs to make a decision and we're kinda stuck in limbo about it. Either treat them as two entirely separate things, or let them be interchangeable. Right now it's a muddy middle ground.

@chibicode

Totally agree with @cbeams:

With that said, @mislav, I understand the decision to deprecate this feature in hub if the writing is on the wall that API v4 will remove transformation altogether. So that's not really at issue here. Is there a better place where we can speak directly to the folks that are responsible for those changes in v4? Perhaps there are motivations we don't know about; perhaps there is a better and more comprehensive workflow for pull requests on the horizon (perhaps something like @wjwwood suggests above). In any case, it should be clear from this discussion that there are plenty of folks who rely on this workflow every day. I'd like to know that the folks responsible for the API are at least aware of this fact, and ideally to hear their rationale for removing it anyway (if indeed they do).

@dopry

-1, I'd like to see this feature remain until github can provide something better. I agree with @jfelchner.

Our teams use git hub issues to manage projects. We open issues for tasks and they get closed with a pull request. I have to open tabs for both the pull request and the issue to do code review. The conversation about the requirements and the implementation are split into two places. Hopefully, everyone remembers to add the magic words closes #NNN. It's a messy and frustrating workflow. I find myself having to keep two tickets in sync for every task my team is working in.

I was looking at converting pull requests as a solution to one of the biggest pain points in my daily workflow. With the future of that feature in flux, I'm stuck bungling though multiple tabs.

@karussell

Technical details aside, GH issues and PRs are suffering from a split of communication streams.

+1

All discussion on 0-N PRs should circle back into the issue.
Removing the ability from PRs to contain any content (aside from code) would be positive.
In turn, removing the ability to convert issues into PRs sounds like a necessary intermediate step to achieve that goal.

Sure.

Another solution would be to have an exclusive list of dependent issues or pull requests or how ever you would call some more prominent issues. E.g. on the right site instead of somewhere in the discussion.

The problem with mixing in discussion from the pull requests is that there it is about the solution detail, not necessarily about the problem anymore. Maybe pull requests should 'implement' an issue where the issue is the interface ;)

@entropyq

Forcing your preferred workflow on others is lame.
I happen to make my issues focused enough that they are the size of pull requests. I create new issues if I need to split up the work. Converting issues to pull-requests when I'm done is simply better for me. Boo on you!

@teubanks

I would rather this feature stayed in general (I understand if it's gone from here because the API doesn't support it).

I also create focused issues that make sense when converted into PRs. But I also work on private repos most of the time. If I were mainly writing on public repos, I probably wouldn't use this feature, but that doesn't mean I'd want it gone

@pglombardo

@github needs to be a good community steward and clarify it's opinion/direction on this issue. This discussion has been going on for ~2 years which is far too long.

@meesterdude

@pglombardo github DGAF. They used to, but now that they have the users they can do what they want. We might still be discussing it, but it's pretty clear its a done deal for them and they have shown no interest in taking up the issue any further. It sucks, but thats life.

@mt33
mt33 replied Jan 22, 2016

Any updates on this? I'm looking to see if this will have long term support or not, I think it's a fantastic feature.

@o1dnik

This is a very useful feature to be able to create a pull-request by not opening the web UI. I would vote (if there's voting anywhere) for proper auth check of an issue ownership rather than removing this from API v4.

@vshjxyz

+1 for keeping it! same process as @wjwwood

Please sign in to comment.