Skip to content

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 24, 2014
Merged

Allow to provide parameters for PullRequests list #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1406c44
Allow to provide parameters for PullRequests list
syphernl Jun 13, 2014
3fbf0dd
Fix tests and add defaults
syphernl Jun 18, 2014
722a46c
Also fix the actual tests since it requires an array now
syphernl Jun 18, 2014
d84b9ff
Tabs vs spaces, missing )
syphernl Jun 19, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions lib/Github/Api/PullRequest.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,16 @@ class PullRequest extends AbstractApi
*
* @param string $username the username
* @param string $repository the repository
* @param string $state the state of the fetched pull requests.
* The API seems to automatically default to 'open'
* @param integer $page the page
* @param integer $perPage the per page
* @param array $params a list of extra parameters.
*
* @return array array of pull requests for the project
*/
public function all($username, $repository, $state = null, $page = 1, $perPage = 30)
public function all($username, $repository, array $params = array())
{
$parameters = array(
'page' => $page,
'per_page' => $perPage,
'state' => $state,
);
$parameters = array_merge(array(
'page' => 1,
'per_page' => 30
), $params);
Copy link
Contributor

@cursedcoder cursedcoder Jun 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add $defaults array containing basic values from constructor and merge it

<?php

$parameters = array_merge(array(
    'page' => 1,
    'per_page' => 30
), $params);


return $this->get('repos/'.rawurlencode($username).'/'.rawurlencode($repository).'/pulls', $parameters);
Copy link
Contributor

@cursedcoder cursedcoder Jun 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be 1 blank line before return statement

}
Expand Down
4 changes: 2 additions & 2 deletions test/Github/Tests/Api/PullRequestTest.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function shouldGetOpenPullRequests()
->with('repos/ezsystems/ezpublish/pulls', array('state' => 'open', 'per_page' => 30, 'page' => 1))
->will($this->returnValue($expectedArray));

$this->assertEquals($expectedArray, $api->all('ezsystems', 'ezpublish', 'open'));
$this->assertEquals($expectedArray, $api->all('ezsystems', 'ezpublish', array('state' => 'open')));
}

/**
Expand All @@ -49,7 +49,7 @@ public function shouldGetClosedPullRequests()
->with('repos/ezsystems/ezpublish/pulls', array('state' => 'closed', 'per_page' => 30, 'page' => 1))
->will($this->returnValue($expectedArray));

$this->assertEquals($expectedArray, $api->all('ezsystems', 'ezpublish', 'closed'));
$this->assertEquals($expectedArray, $api->all('ezsystems', 'ezpublish', array('state' => 'closed')));
}

/**
Expand Down