browse -b flag #258

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
6 participants

jianli commented Nov 10, 2012

hub browse -b opens the URL of the pull request associated with your current branch.
hub browse -b foo opens the URL of the pull request associated with branch foo.

This is my first time coding Ruby, so feedback is appreciated!

@jianli jianli referenced this pull request Nov 10, 2012

Closed

pull-request -l flag #255

Member

mislav commented Nov 13, 2012

I'm curious as to why you needed this functionality?

The Ruby code is not bad, although you should be writing tests for new features, but I don't see how this command fits in the workflow.

The other trouble I see with it that it transforms the old pull-request command into something else. The original pull-request command makes a pull requests, while your pull-request -l just prints data. It's an entirely different feature embedded in the same command, which is against unix philosophy.

jianli commented Nov 13, 2012

I'm curious as to why you needed this functionality?

This flag has actually been quite useful to me; I have a workflow where I have several feature branches and pull requests active at the same time, so I need a quick way to get the URL of the pull request given a feature branch. For example,

open -a 'Google Chrome' `hub pull-request -l`

There is nothing in the Github web interface which maps feature branches to pull requests, so it's only through the API that this can be achieved.

The other trouble I see with it that it transforms the old pull-request command into something else. The original pull-request command makes a pull requests, while your pull-request -l just prints data. It's an entirely different feature embedded in the same command, which is against unix philosophy.

Would you suggest a new command? I couldn't think of a good name other than pull-request.

My original rationale was for overloading pull-request was this: The original pull-request actually prints the URL as well after successfully creating the pull request. But if you run pull-request for a feature branch which already has one, hub gives the unhelpful "Error creating pull request: Unprocessable Entity (HTTP 422)". So this patch seems to be an improvement in that it actually prints the specific pull request if it exists.

I guess a cleaner way would be to always check that there is an existing pull request right before creating one, though that would have performance implications for the current use case.

jianli commented Nov 28, 2012

I wrote some tests.

Member

mislav commented Dec 1, 2012

Everything looks fine. I can see how this can be useful. However I won't pull this just yet because I don't like the fact that we're overloading pull-request.

I have an idea of splitting hub into multiple smaller scripts that follow the unix philosophy. A command that provides the thing you need would fit there.

jianli commented Dec 2, 2012

Here's a proposal:

It seems that the current behavior of hub pull-request is

  1. Try to create a pull request. If one currently exists, print a 422 error message.
  2. Otherwise, create a new pull request and print its name.

What if we made the behavior

  1. Check if a pull-request exists. If so, print its name
  2. Otherwise, create a new pull request and print its name

I know this won't solve the issue of overloading pull-request, but it seems more elegant to me.

we already have hub browse, that's what we should overload (i think).

👍 for adding this to hub browse:

From the man page:
https://github.com/defunkt/hub/blob/master/man/hub.1#L396-L415

It looks like it belongs there.

jianli commented Jun 12, 2013

Ok, I've moved this command to hub browse and updated the PR description up top.

There is (still) a test for this feature. It seems that one or more of hub.1, hub.1.html, hub.1.ronn, and README.md is automatically generated from the others, so I don't know how to proceed with adding documentation in those places.

Member

mislav commented Jun 12, 2013

This is interesting functionality. I'm planning to add a command that lists open pull requests. This seems to belong to this new command, but on the other hand it seems like it belongs to browse as well. I need to think about this.

emarref commented Jun 13, 2013

Just wanted to chime in here to say I'm very interested in being able to view the pull request related to a branch also. Looking forward to this feature.

jianli commented May 8, 2014

I've realized that it's possible to do this by simply navigating to github.com/user/repo/pull/branch_name, which bypasses the API calls made in this PR.

This is what I currently use (zsh function):

function pr
{
    url=`hub browse -u`
    echo ${url/tree/pull}
    open ${url/tree/pull}
}
Member

mislav commented May 11, 2014

@jianli Yes I've just discovered this last week as well. It's a great feature. I'm thinking of baking it in the next version of hub, but we still have to decide on the command syntax. I don't want to overload the browse command any more.

aik099 commented May 11, 2014

The browse command is the real time save for me. Thanks.

Member

mislav commented Oct 21, 2014

Closing this PR as it won't apply anymore since we nuked the Ruby implementation and replaced it with Go: #642. Sorry that this hasn't made it in. We plan to improve the browse command in future releases, though.

@mislav mislav closed this Oct 21, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment