Skip to content

Add support for fetching statuses #565

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 7 commits into from
Apr 14, 2017
+57 −16
Merged

Add support for fetching statuses #565

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
59423cd
Add support for fetching statuses
slaughter550 Apr 11, 2017
a871349
fix style ci
slaughter550 Apr 12, 2017
d6b0352
Add docs, add test, linter cleanup
slaughter550 Apr 13, 2017
41a631e
Merge in master
slaughter550 Apr 13, 2017
50617f0
Merge branch 'master' into patch-3
slaughter550 Apr 13, 2017
34959f7
Update language
slaughter550 Apr 13, 2017
2f3cda7
Fix test
slaughter550 Apr 13, 2017
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
7 changes: 7 additions & 0 deletions lib/Github/Api/PullRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

@Nyholm Nyholm Apr 12, 2017

Choose a reason for hiding this comment

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

Could you add method doc here? And also a link to the github documentation?

Copy link
Contributor Author

@slaughter550 slaughter550 Apr 12, 2017

Choose a reason for hiding this comment

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

yeah I didn't see it for the ones above so I wasn't sure if you were requiring it or not

Copy link
Collaborator

@Nyholm Nyholm Apr 12, 2017

Choose a reason for hiding this comment

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

Please do add it.

{
$link = $this->show($username, $repository, $id)['_links']['statuses']['href'];
Copy link
Collaborator

@Nyholm Nyholm Apr 12, 2017

Choose a reason for hiding this comment

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

Ah. So this is just an helper method?

Copy link
Contributor Author

@slaughter550 slaughter550 Apr 12, 2017

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@Nyholm Nyholm Apr 14, 2017

Choose a reason for hiding this comment

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

What would happen if $this->show() failed?

Copy link
Contributor Author

@slaughter550 slaughter550 Apr 14, 2017

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@Nyholm Nyholm Apr 14, 2017

Choose a reason for hiding this comment

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

Good


return $this->get($link);
}

public function comments()
{
return new Comments($this->client);
Expand Down