Conversation
|
Excellent. Thank you for this PR. I see that it is missing some tests. Would you add some and I'll do a review of this. |
|
@Nyholm Thank you! |
[ci skip] [skip ci]
|
Fixed everything and checked it works! |
| * | ||
| * @return array | ||
| */ | ||
| public function views($owner, $repository) |
There was a problem hiding this comment.
Missing parameter per. See https://developer.github.com/v3/repos/traffic/#views
| * | ||
| * @return array | ||
| */ | ||
| public function clones($owner, $repository) |
There was a problem hiding this comment.
Missing parameter per. See https://developer.github.com/v3/repos/traffic/#clones
|
I've created a PR to explain how to write tests. See #503 Is it understandable? Should I add something to it so it makes more sense? |
[ci skip] [skip ci]
|
@Nyholm Replied in your PR. |
Nyholm
left a comment
Nyholm
left a comment
There was a problem hiding this comment.
I like this PR. Just make sure to add the per parameters and write some proper tests and I'll be happy to merge.
|
|
||
| class TrafficTest extends TestCase | ||
| { | ||
| // ... |
| /** | ||
| * @test | ||
| */ | ||
| public function shoulddoSomething() |
There was a problem hiding this comment.
Please update the name of this function
There was a problem hiding this comment.
You should also update the test content
|
@Nyholm As I said earier, I don't know hot to write tests... |
Tests specific to this library or in general? Read about how to write tests to the Github library here: https://github.com/KnpLabs/php-github-api/blob/master/doc/testing.md Testing in general: https://www.startutorial.com/articles/view/phpunit-beginner-part-1-get-started |
Nyholm
left a comment
Nyholm
left a comment
There was a problem hiding this comment.
I cannot merge this without tests. Will you work on the tests?
| * | ||
| * @return array | ||
| */ | ||
| public function clones($owner, $repository) |
|
@Nyholm Oops! I deleted the repo... |
|
I do not think so. Sorry. |
|
I added this to the milestone 2.2. Is it possible that you can add tests to this PR this weekend? |
|
@Nyholm I'll try |
2 participants
Fix #498
Add support for the traffic API.
https://developer.github.com/v3/repos/traffic/