-
-
Notifications
You must be signed in to change notification settings - Fork 600
Allow to provide parameters for PullRequests list #136
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
Conversation
According to the [API documentation](https://developer.github.com/v3/pulls/#list-pull-requests) of the v3 API a sort can be added and some other parameters not yet supported by this library. I have changed the "all" function to allow parameters to be given.
tests are failing, could you please fix it? |
); | ||
|
||
return $this->get('repos/'.rawurlencode($username).'/'.rawurlencode($repository).'/pulls', $parameters); | ||
return $this->get('repos/'.rawurlencode($username).'/'.rawurlencode($repository).'/pulls', $params ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove space after variable params
As requested / suggested. |
are you sure?
|
$parameters = array_merge(array( | ||
'page' => 1, | ||
'per_page' => 30 | ||
), $params); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove extra spacing
It looks like this is now ready to be merged @cursedcoder |
very nice @syphernl |
Allow to provide parameters for PullRequests list
@cursedcoder merging a BC break and releasing it in a patch-level release (1.2.4 in this case) is really not nice for people depending on your library. Please follow semver |
@stof ok |
According to the API documentation of the v3 API a sort can be added and some other parameters not yet supported by this library. I have changed the "all" function to allow parameters to be given.