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

add a command for listing all open pull requests #351

Closed
wants to merge 1 commit into from

Conversation

radamant
Copy link

@radamant radamant commented Jul 9, 2013

You can run:

hub pulls

And get output like:

jm3 - Add correct hub alias for fish shell (fixes #248)
https://github.com/defunkt/hub/pull/350

pchw - add pull-request to open-browser option
https://github.com/defunkt/hub/pull/344

mmozuras - Figure out default branch for the project instead of just using master
https://github.com/defunkt/hub/pull/326

timmow - Add info on using with Github Enterprise
https://github.com/defunkt/hub/pull/297

jianli - browse -b flag
https://github.com/defunkt/hub/pull/258

You can run:

`hub pulls`

And get output like:

```
jm3 - Add correct hub alias for fish shell (fixes mislav#248)
mislav#350

pchw - add pull-request to open-browser option
mislav#344

mmozuras - Figure out default branch for the project instead of just using master
mislav#326

timmow - Add info on using with Github Enterprise
mislav#297

jianli - browse -b flag
mislav#258
```
@mislav
Copy link
Owner

mislav commented Jul 15, 2013

Hi @radamant

This looks good, although I'd like to have this with caching. I realize this complicates implementation a bit, but would lead to a much better experience.

I would cache pull requests from every GitHub API response and the next time I'm requesting pulls, I would only request changes after the timestamp when cache was last made.

Also, I'm not writing new unit tests anymore. All new tests should go under features/ as Cucumber stories.

I was thinking of working on this myself. I'll keep this open until I either merge it or implement something equivalent. Thanks for your help!

@radamant
Copy link
Author

@mislav is there anything I can do to help get this merged? This feature is something my current team would use frequently and they are currently using my fork rather than canonical hub.

@mislav
Copy link
Owner

mislav commented Jul 15, 2013

To ship this feature, it needs:

  1. Pagination to fetch all pull requests, not just the first 30 (API default)

  2. Caching to disk because paginating every time for large repos is going to be wasteful

  3. Conditional GET of only pull requests that have changed since last time they were fetched

  4. Serving pulls from disk cache if requesting them from API fails (offline mode)

  5. Changing the format of the output:

    {PULL ID}: This is the pull request title - Author's Full Name ({USERNAME})
    
  6. Not outputting URLs in the output. It doesn't make sense.

  7. Testing the functionality with Cucumber stories instead of unit tests.

Bonus features:

  1. Possibly making the output format configurable, similar to git log format
  2. Indicating whether a pull request has green status on CI.

I don't expect you to do all this work. I'm going to do as much as I can, but I'll leave your contribution unmerged and open until I decide just how much (if any) of your code I will use. Your coworkers should in the meantime use your fork.

@leoj3n
Copy link

leoj3n commented Jul 30, 2013

@radamant I have no affiliation, but you might be interested in https://github.com/eduardolundgren/node-gh

@davidlt
Copy link

davidlt commented Sep 4, 2013

ping.

@mislav
Copy link
Owner

mislav commented Sep 4, 2013

No sense in pinging me :) I still didn't have time to implement this, but I will at some point. Everyone is welcome to beat me to it, if they take in consideration my thoughts above

@bantu
Copy link

bantu commented Sep 23, 2013

Consider adding an option for limiting the list to those pull requests that need conflict resolution on merge. Not sure whether and how this can be detected using the Github API.

@mislav mislav mentioned this pull request Feb 5, 2014
3 tasks
@mislav
Copy link
Owner

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. It's on our roadmap to provide a feature for listing issues/pulls, though. Stay tuned.

@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants