Primed solving of https://github.com/ornicar/php-github-api/issues/13, implementation of the PullRequest API. #14

Merged
merged 6 commits into from Jun 9, 2011

3 participants

@jeanvoye

Primed solving of #13,
based on Github's Pull Request API documentation : http://develop.github.com/p/pulls.html

A bunch of TODOs left, namely regarding return values from Github's API, which should probably be handled through exceptions. I could not see much analysis of the return values, did not dare taking a revamping initiative, but will gladly bring additions if you think this makes sense.

Thanks for this nice API,
Nicolas

Nicolas Pastorino Primed solving of ornicar#13,
based on Github's Pull Request API documentation : http://develop.github.com/p/pulls.html
3700336
@ornicar ornicar commented on the diff Jun 8, 2011
lib/Github/Api/PullRequest.php
+ * @param string $base A String of the branch or commit SHA that you want your changes to be pulled to.
+ * @param string $head A String of the branch or commit SHA of your changes.
+ * Typically this will be a branch. If the branch is in a fork of the original repository,
+ * specify the username first: "my-user:some-branch".
+ * @param string $title The String title of the Pull Request. Used in pair with $body.
+ * @param string $body The String body of the Pull Request. Used in pair with $title.
+ * @param int $issueId If a pull-request is related to an issue, place issue ID here. The $title-$body pair and this are mutually exclusive.
+ * @return array array of pull requests for the project
+ */
+ public function create($username, $repo, $base, $head, $title = null, $body = null, $issueId = null)
+ {
+ $postParameters = array( 'pull[base]' => $base,
+ 'pull[head]' => $head
+ );
+
+ if ( $title !== null and $body !== null )
@ornicar
Owner
ornicar added a line comment Jun 8, 2011

Coding standards: the bracket opens on the same line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ornicar ornicar commented on the diff Jun 8, 2011
lib/Github/Api/PullRequest.php
+ */
+ public function create($username, $repo, $base, $head, $title = null, $body = null, $issueId = null)
+ {
+ $postParameters = array( 'pull[base]' => $base,
+ 'pull[head]' => $head
+ );
+
+ if ( $title !== null and $body !== null )
+ {
+ $postParameters = array_merge( $postParameters,
+ array(
+ 'pull[title]' => $title,
+ 'pull[body]' => $body
+ )
+ );
+ }
@ornicar
Owner
ornicar added a line comment Jun 8, 2011

CS:

if () {
} elseif () {
} else {
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ornicar ornicar commented on an outdated diff Jun 8, 2011
lib/Github/Api/PullRequest.php
+ 'pull[body]' => $body
+ )
+ );
+ }
+ elseif ( $issueId !== null )
+ {
+ $postParameters = array_merge( $postParameters,
+ array(
+ 'pull[issue]' => $issueId
+ )
+ );
+ }
+ else
+ {
+ // @FIXME : Exception required here.
+ return null;
@ornicar
Owner
ornicar added a line comment Jun 8, 2011

I think we can have a lib/Github/Exception dir with Github_Exception extends Exception class, and maybe more precise exception classes. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ornicar ornicar commented on the diff Jun 8, 2011
test/Github/Tests/Api/PullRequestTest.php
+ ->method('post')
+ ->with('pulls/ezsystems/ezpublish',
+ array( 'pull[base]' => 'master',
+ 'pull[head]' => 'virtualtestbranch',
+ 'pull[issue]' => 25
+ )
+ );
+
+ $api->create('ezsystems', 'ezpublish', 'master', 'virtualtestbranch', null, null, 25 );
+ }
+
+ protected function getApiClass()
+ {
+ return 'Github_Api_PullRequest';
+ }
+}
@ornicar
Owner
ornicar added a line comment Jun 8, 2011

Oh yeah, I love tested pull requests :)

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

Great stuff ! Care to write the corresponding doc?

@nfrp

Updated CS + updated doc + an addition forgotten in the initial PR are now part of this pull request.
Regarding the Exceptions discussion : probably something that should be added at the heart of this library, not only in specific places. How about opening a separate issue for this topic, where discussion can safely occur ?

Cheers,

@nfrp

I realize i pushed the last comment with my professional github account. But this is @jeanvoye too :)

@ornicar ornicar merged commit c8b366c into ornicar:master Jun 9, 2011
@ornicar
Owner

Great job, thanks!

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