Status API #83
Status API #83
Conversation
| .execFuture(headerUserAgent) | ||
|
|
||
| testFutureIsLeft(response) | ||
| } |
There was a problem hiding this comment.
I don't know how you want to deal with integration tests for the status API.
There was a problem hiding this comment.
We've tested it locally, we know it fails with fork but it should work once it's merged
| description: Option[String], | ||
| fork: Boolean, | ||
| urls: Map[String, String] | ||
| ) |
There was a problem hiding this comment.
It doesn't really fit into the existing Repository case class so I chose to create a new one, input welcome.
There was a problem hiding this comment.
it makes sense to me to have a different model. I'm wondering if we could just add the fields url and html_url instead of the urls map. In this way, we could avoid the custom decoder.
There was a problem hiding this comment.
I don't think we can get rid of the custom decoder since we have all those urls to deal with.
|
Thanks @BenFradet, great contribution :) we'll review it these upcoming days. |
| description: Option[String], | ||
| fork: Boolean, | ||
| urls: Map[String, String] | ||
| ) |
There was a problem hiding this comment.
it makes sense to me to have a different model. I'm wondering if we could just add the fields url and html_url instead of the urls map. In this way, we could avoid the custom decoder.
| .createStatus( | ||
| "BenFradet", | ||
| "sparkml-als", | ||
| "ae6eb4ae126f29d96249a2393c633c8d91a4adf4", |
There was a problem hiding this comment.
Please, change the first three params with validRepoOwner, validRepoName, and validCommitSha. We've tested it locally with the Travis token so it should pass once we merged the PR
| val response = Github(accessToken).statuses | ||
| .createStatus( | ||
| "BenFradet", | ||
| "sparkml-als", |
There was a problem hiding this comment.
Please, change the first two params with validRepoOwner, and validRepoName.
| .execFuture(headerUserAgent) | ||
|
|
||
| testFutureIsLeft(response) | ||
| } |
There was a problem hiding this comment.
We've tested it locally, we know it fails with fork but it should work once it's merged
| .createStatus( | ||
| "BenFradet", | ||
| "sparkml-als", | ||
| "ae6eb4ae126f29d96249a2393c633c8d91a4adf4", |
There was a problem hiding this comment.
Please, change the first three params with validRepoOwner, validRepoName, and validCommitSha.
| val response = Github(accessToken).statuses | ||
| .createStatus( | ||
| "BenFradet", | ||
| "sparkml-als", |
There was a problem hiding this comment.
Please, change the first two params with validRepoOwner, and validRepoName.
| @@ -543,7 +543,7 @@ trait FakeResponses { | |||
| |} | |||
| """.stripMargin | |||
|
|
|||
| val unauthorizedReponse = | |||
| val unauthorizedResponse = | |||
|
Changed the integration tests, thanks for the review 👍 |
|
Thanks a lot, looking forward to the next release 👍 |
No description provided.