Skip to content
This repository has been archived by the owner on Mar 3, 2022. It is now read-only.

Add support for pull requests #288

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shubhamshuklaer
Copy link
Collaborator

The code belongs to @LFDM. I just rebased it to master, fixing conflicts. The
pull request #158 was closed because of merge conflicts. Fixes issue #256.
Because of other reservations I was not able to test it extensively, but with
basic testing its working. I would be grateful if someone can test it. You can
test it using the link
https://raw.githubusercontent.com/shubhamshuklaer/ghi/pulls_fix/ghi .

@AlexChesters
Copy link
Contributor

AlexChesters commented Apr 19, 2016

Hi @shubhamshuklaer!

Thanks for the PR, would you mind squashing all of the commits into one please?

Thanks!

@AlexChesters
Copy link
Contributor

Obviously as this is a substantial PR I don't feel I have the ability to properly review it, I'll have to leave that down to @stephencelis

@stephencelis
Copy link
Owner

stephencelis commented Apr 21, 2016

I might not have time to look over this big of a diff for awhile. From what I remember, @LFDM's pull was great, but I encountered a few minor bugs. These cases may have been unrelated and subsequently fixed, though. I think stress testing this PR will give us some confidence. I encourage all ghi users interested in PR support to pull down this version and test it within their normal workflow, reporting bugs (or lack thereof) back here.

@shubhamshuklaer
Copy link
Collaborator Author

Fixed conflicts from #292

@stephencelis
Copy link
Owner

@shubhamshuklaer (and anyone else who's following this branch) Have you been running this exclusively and found things to be relatively stable?

@shubhamshuklaer
Copy link
Collaborator Author

Well I don't get to use pull request that often as I don't manage any big repo. I will see if I can test it this week.

@shubhamshuklaer
Copy link
Collaborator Author

shubhamshuklaer commented May 29, 2016

Though the tests in #308 are not very exhaustive(We need more tests). But I tested by creating a branch off of this(#288) and then merging #308 into it. All test passed.
https://travis-ci.org/shubhamshuklaer/ghi/builds/133670481
https://github.com/shubhamshuklaer/ghi/tree/pulls_travis_tmp

Though #308 doesn't contain any test for pull request. But at-least it means it won't(most probably, need more tests) cause a regression.

Once #308 is merged, I will try adding tests for this by rebasing it off of master.

@stephencelis stephencelis removed their assignment Nov 1, 2016
@stephencelis
Copy link
Owner

I unfortunately don't have the bandwidth to manage this right now. Feel free to move ahead with thorough testing, but be mindful of our users before release!

@sukima
Copy link

sukima commented Jan 12, 2017

Poke

@emoshaya
Copy link

emoshaya commented Apr 8, 2017

will this change also allow you to assign Reviewers? or just Assignees?

@drazisil
Copy link

drazisil commented Oct 3, 2017

@timofonic Do you know if it has been tested? I'm not good enough with Ruby to blindly merge it into my fork.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants