Arf 105 create pull request api #109

AdrianRaFo · 2017-05-02T10:17:48Z

Fixes #105 Create PullRequest Api integration

fedefernandez

Minor comments but we should add the docs for these two new operations here

fedefernandez · 2017-05-02T10:34:02Z

github4s/jvm/src/test/scala/github4s/utils/TestUtils.scala

+  val invalidNewPullRequestData = NewPullRequestData("", "")
+
+  val validNewPullRequestIssue   = NewPullRequestIssue(26)
+  val invalidNewPullRequestIssue = NewPullRequestIssue(5)


The invalid vals are not used, could you please remove them?

fedefernandez · 2017-05-02T10:34:54Z

github4s/shared/src/main/scala/github4s/api/PullRequests.scala

+   *                             requests in the same network, namespace head with a user like this: username:branch.
+   * @param base		Required. The name of the branch you want the changes pulled into. This should be an existing branch
+   *                             on the current repository. You cannot submit a pull request to one repository that
+   * @param maintainerCanModify		Indicates whether maintainers can modify the pull request.


Bad indentation, please remove the tabs in the scaladocs. Also, please specify that the value for maintainerCanModify is true by default

fedefernandez · 2017-05-02T10:41:37Z

github4s/shared/src/main/scala/github4s/api/PullRequests.scala

+              base,
+              maintainerCanModify).asJson.noSpaces)
+
+    }


I think it's more clear to create the data first and then make the call, something like:

def create( accessToken: Option[String] = None, headers: Map[String, String] = Map(), owner: String, repo: String, newPullRequest: NewPullRequest, head: String, base: String, maintainerCanModify: Option[Boolean] = Some(true)): M[GHResponse[PullRequest]] = { val data = newPullRequest match { case NewPullRequestData(title, body) ⇒ CreatePullRequestData(title, head, base, body, maintainerCanModify) case NewPullRequestIssue(issue) ⇒ CreatePullRequestIssue(issue, head, base, maintainerCanModify) httpClient.post[PullRequest](accessToken, s"repos/$owner/$repo/pulls", headers, data.asJson.noSpaces) }

fedefernandez · 2017-05-02T10:51:54Z

Also, please add the tests in ApiSpec.scala

Update pull_request.md

…overage Fixes a field name in `CreatePullRequest` and increases test coverage

codecov-io · 2017-05-03T08:54:20Z

Codecov Report

Merging #109 into master will decrease coverage by 0.12%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master     #109      +/-   ##
==========================================
- Coverage   88.14%   88.01%   -0.13%     
==========================================
  Files          36       36              
  Lines         506      534      +28     
  Branches        3        3              
==========================================
+ Hits          446      470      +24     
- Misses         60       64       +4

Impacted Files	Coverage Δ
.../main/scala/github4s/free/domain/PullRequest.scala	`100% <ø> (ø)`	⬆️
...cala/github4s/free/interpreters/Interpreters.scala	`83.87% <0%> (-1.38%)`	⬇️
...n/scala/github4s/free/algebra/PullRequestOps.scala	`60% <0%> (-40%)`	⬇️
...4s/shared/src/main/scala/github4s/GithubAPIs.scala	`96.77% <100%> (+0.1%)`	⬆️
...red/src/main/scala/github4s/api/PullRequests.scala	`100% <100%> (ø)`	⬆️
...ub4s/shared/src/main/scala/github4s/Encoders.scala	`100% <100%> (ø)`	⬆️
...4s/shared/src/main/scala/github4s/HttpClient.scala	`85.1% <0%> (+6.53%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b869e17...d8f680f. Read the comment docs.

juanpedromoreno · 2017-05-03T08:52:13Z

docs/src/main/tut/pull_request.md

+
+## Create a pull request
+If you want to create a pull request,we have two ways to create a pull request.


--pull request,we have ++pull request, we have

juanpedromoreno · 2017-05-03T08:56:52Z

docs/src/main/tut/pull_request.md

+```
+
+On the other hand, we can pass a `issue` id (in `NewPullRequestIssue` object) instead of title and body to get this parameter of the issue


--we can pass a `issue` ++we can pass an `issue`

-- (in `NewPullRequestIssue` object) ++ (as part of the `NewPullRequestIssue` object)

-- instead of title and body to get this parameter of the issue ++ instead of the title and the body to get this parameter of the issue.

juanpedromoreno · 2017-05-03T08:59:36Z

docs/src/main/tut/pull_request.md

+On the other hand, we can pass a `issue` id (in `NewPullRequestIssue` object) instead of title and body to get this parameter of the issue
+
+**NOTE**: This option delete the issue


It should end with full stop char ..

-- This option delete ++ This option deletes

juanpedromoreno · 2017-05-03T09:01:12Z

github4s/js/src/test/scala/github4s/unit/DecodersSpec.scala

+    decode[Repository](emptyListResponse).isLeft shouldBe true
+  }
+
+  "StatusRepository decoder" should "return a valid repo for a valid JSON" in {


CombinedStatus decoder

juanpedromoreno · 2017-05-03T09:04:09Z

github4s/jvm/src/test/scala/github4s/unit/ApiSpec.scala

@@ -608,11 +608,59 @@ class ApiSpec
      accessToken,
      headerUserAgent,
      validRepoOwner,
-      invalidRepoName,
+      validPRRepoName,


Either the test banner or this value are wrong. We should make this consistent.

juanpedromoreno · 2017-05-03T09:06:00Z

github4s/jvm/src/test/scala/github4s/utils/TestUtils.scala

@@ -111,16 +111,33 @@ trait TestUtils {
  val validPullRequestFileSha = "f80f79cafbe3f2ba71311b82e1171e73bd37a470"
  val validPullRequestNumber  = 1

+  val validPRRepoName = "sbt-dependencies-test"


What would happen if this repository is deleted?

We could use github4s

… into arf-105-Create-Pull-Request-API # Conflicts: # github4s/jvm/src/test/scala/github4s/unit/ApiSpec.scala

juanpedromoreno

AdrianRaFo added 6 commits April 28, 2017 13:05

Support the Create Pull Request API

ab81588

Create jvm test

c69665c

Create js jvm and unit test

572981c

Delete test

e39ee36

Create unit test

9351564

Create unit test util

a183c11

fedefernandez requested changes May 2, 2017

View reviewed changes

AdrianRaFo and others added 11 commits May 2, 2017 15:22

Create unit test util

5633a82

Doc update

6da35b9

ScalaDoc update

9a6961d

Doc update

d51af0d

Doc update

3974527

Doc update

46cb8c1

Update pull_request.md

bdceeab

Merge pull request #110 from 47deg/fedefernandez-patch-2

95ac4b9

Update pull_request.md

Doc update

a7a6132

Fixes a field name in CreatePullRequest and increases test coverage

92846d6

Merge pull request #111 from 47deg/ff-fixes-field-and-increase-test-c…

11868ca

…overage Fixes a field name in `CreatePullRequest` and increases test coverage

juanpedromoreno requested changes May 3, 2017

View reviewed changes

AdrianRaFo added 5 commits May 3, 2017 11:17

Update pull_request.md

feaa3f7

Update DecodersSpec.scala

440f4fd

Update ApiSpec.scala

a66dc83

delete validPRRepoName

6973ad6

Merge remote-tracking branch 'origin/arf-105-Create-Pull-Request-API'…

d8f680f

… into arf-105-Create-Pull-Request-API # Conflicts: # github4s/jvm/src/test/scala/github4s/unit/ApiSpec.scala

juanpedromoreno approved these changes May 3, 2017

View reviewed changes

fedefernandez approved these changes May 3, 2017

View reviewed changes

juanpedromoreno merged commit 9ee5cc3 into master May 3, 2017

juanpedromoreno deleted the arf-105-Create-Pull-Request-API branch May 3, 2017 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arf 105 create pull request api #109

Arf 105 create pull request api #109

AdrianRaFo commented May 2, 2017

fedefernandez left a comment

fedefernandez May 2, 2017

fedefernandez May 2, 2017

fedefernandez May 2, 2017

fedefernandez commented May 2, 2017 •

edited

codecov-io commented May 3, 2017 •

edited

juanpedromoreno May 3, 2017

juanpedromoreno May 3, 2017

juanpedromoreno May 3, 2017

juanpedromoreno May 3, 2017

juanpedromoreno May 3, 2017

juanpedromoreno May 3, 2017

fedefernandez May 3, 2017

juanpedromoreno left a comment


		## Create a pull request
		If you want to create a pull request,we have two ways to create a pull request.

		```

		On the other hand, we can pass a `issue` id (in `NewPullRequestIssue` object) instead of title and body to get this parameter of the issue

		On the other hand, we can pass a `issue` id (in `NewPullRequestIssue` object) instead of title and body to get this parameter of the issue

		NOTE: This option delete the issue

Arf 105 create pull request api #109

Arf 105 create pull request api #109

Conversation

AdrianRaFo commented May 2, 2017

fedefernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedefernandez commented May 2, 2017 • edited

codecov-io commented May 3, 2017 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juanpedromoreno left a comment

Choose a reason for hiding this comment

fedefernandez commented May 2, 2017 •

edited

codecov-io commented May 3, 2017 •

edited