Add support for fetching statuses #565
Conversation
lib/Github/Api/PullRequest.php
Outdated
| @@ -58,6 +58,13 @@ public function files($username, $repository, $id) | |||
| return $this->get('repos/'.rawurlencode($username).'/'.rawurlencode($repository).'/pulls/'.rawurlencode($id).'/files'); | |||
| } | |||
|
|
|||
| public function status($username, $repository, $id) | |||
There was a problem hiding this comment.
Could you add method doc here? And also a link to the github documentation?
There was a problem hiding this comment.
yeah I didn't see it for the ones above so I wasn't sure if you were requiring it or not
lib/Github/Api/PullRequest.php
Outdated
| @@ -58,6 +58,13 @@ public function files($username, $repository, $id) | |||
| return $this->get('repos/'.rawurlencode($username).'/'.rawurlencode($repository).'/pulls/'.rawurlencode($id).'/files'); | |||
| } | |||
|
|
|||
| public function status($username, $repository, $id) | |||
| { | |||
| $link = $this->show($username, $repository, $id)['_links']['statuses']['href']; | |||
There was a problem hiding this comment.
Ah. So this is just an helper method?
There was a problem hiding this comment.
Yeah it just to help get the status response for a PR since its URL is built from a SHA instead of the PR Number like all of the other URLs
There was a problem hiding this comment.
Im not sure we want something in the API client that is not an endpoint in the API server.
However, I understand that that this function helps DX. What does @cursedcoder coder think?
Before this PR can be merged we need a unit test that proves correctness.
lib/Github/Api/PullRequest.php
Outdated
| @@ -58,6 +58,13 @@ public function files($username, $repository, $id) | |||
| return $this->get('repos/'.rawurlencode($username).'/'.rawurlencode($repository).'/pulls/'.rawurlencode($id).'/files'); | |||
| } | |||
|
|
|||
| public function status($username, $repository, $id) | |||
|
@Nyholm seems legit for me, if it doesn't influence the code too much and takes only few strings it makes sense |
|
Good. @slaughter550 could you address my comments and make sure the build is green? |
|
@Nyholm addressed feedback |
| */ | ||
| public function status($username, $repository, $id) | ||
| { | ||
| $link = $this->show($username, $repository, $id)['_links']['statuses']['href']; |
There was a problem hiding this comment.
What would happen if $this->show() failed?
There was a problem hiding this comment.
It would throw an http error same as it would if you called that method directly. It either passes and returns the given response or it fails and the whole chain blows up. According the the GitHub api those array keys are guaranteed
|
I made the style changes because the StyleCI was complaining about a bunch of stuff and it was unclear what was a suggestion and what was making it fail. |
|
Okey, I see. Thank you for this PR. |
StyleCI never makes soft suggestions. If it ever suggests something, it can be applied, in fact, applied automatically. It is capable of PRing/committing the changes on its own. |
|
@GrahamCampbell thanks for the info! Still new to StyleCI @Nyholm how often do you guys do version releases? Also when I try to install the v2.x packages I get this Is that expected? |
|
We have no outlined release plan. I was waiting to release 2.3 because it was some PR I really wanted to include. But I think we are all done now, I'll see to a release today. About the installation errors. Have a look in our readme: https://github.com/KnpLabs/php-github-api#autoload and https://github.com/KnpLabs/php-github-api/blob/master/doc/customize.md |
No description provided.