Adding Deployment API - For Review #174
Conversation
| public function all($username, $repository, array $params = array()) | ||
| { | ||
| return $this->get('repos/'.rawurlencode($username).'/'.rawurlencode($repository).'/deployments', array(), | ||
| ['Accept' => 'application/vnd.github.cannonball-preview+json'] |
There was a problem hiding this comment.
you should not use the short array syntax, as it is not compatible with PHP 5.3
|
Thank you for contribution :) . But this PR is missing for:
|
|
@ericduran Are you open to contributions to this PR? Would be happy to look into submitting a PR or two to https://github.com/ericduran/php-github-api/tree/deploymentapi in order to add some unit tests. |
|
@lol768 sure, I'm getting back into this next week so if you can get to it before me awesome :) currently distracted with the status api. Ha. |
|
@ericduran Awesome! I've opened a PR to your fork over here, let me know your thoughts there. The status API does look pretty neat. Edit: merged. @pilot Any thoughts on this? |
|
@lol768 ha just updated to the latest master it already has the status api, looking at the PR now. |
|
@lol768 Ha, thanks for all that clean up and well everything. So we can keep the same PR and the conversation going I just went ahead and added you as a collab on my repo so if you need to make more changes based on any feedback you can just do that without requiring me to just merge PRs. That being said this looks pretty good 👍 @Nek- thoughts? Would you like the commits cleans up? or is that fine? I like keeping the history since there are multiple contributors here. |
|
@ericduran if you want to squash commits to clean the history, only squash commits which belong to the same author. Don't squash together commits with different authorship |
|
Merge conflicts resolved. Given other PRs have been pulled with multiple commits, I'm not sure there's a lot to be gained by squashing everything. If it becomes apparent that squashing the commits is necessary for the PR to be pulled, this can be looked into. At this stage it'd be nice to hear some thoughts about whether the PR can be pulled and if not, what can be done to get it to the stage where it is ready to be pulled. |
| public function shouldCreateDeployment() | ||
| { | ||
| $api = $this->getApiMock(); | ||
| $deploymentData = array("ref" => "fd6a5f9e5a430dddae8d6a8ea378f913d3a766f9"); |
|
Updated per commit line comment. |
Adding Deployment API - For Review
|
Thanks! |
This is still a work in progress but I figure I'll open up this PR now for comments, reviews, concern, etc..