Skip to content
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.

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

Support Comment API #127

Merged
merged 18 commits into from
May 11, 2017
Merged

Support Comment API #127

merged 18 commits into from
May 11, 2017

Conversation

AdrianRaFo
Copy link
Contributor

Fixes #106 Create, Edit and Delete Comments

@juanpedromoreno juanpedromoreno changed the title Support comment api Support Comment API May 10, 2017
@codecov-io
Copy link

codecov-io commented May 10, 2017

Codecov Report

Merging #127 into master will decrease coverage by 0.33%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #127      +/-   ##
=========================================
- Coverage   88.44%   88.1%   -0.34%     
=========================================
  Files          38      38              
  Lines         528     555      +27     
  Branches        2       2              
=========================================
+ Hits          467     489      +22     
- Misses         61      66       +5
Impacted Files Coverage Δ
...cala/github4s/free/interpreters/Interpreters.scala 80% <0%> (-3.59%) ⬇️
...c/main/scala/github4s/free/algebra/IssuesOps.scala 50% <0%> (-21.43%) ⬇️
...cala/github4s/HttpRequestBuilderExtensionJVM.scala 100% <100%> (ø) ⬆️
...4s/shared/src/main/scala/github4s/HttpClient.scala 100% <100%> (+10.34%) ⬆️
...4s/shared/src/main/scala/github4s/api/Issues.scala 100% <100%> (ø) ⬆️
...4s/shared/src/main/scala/github4s/GithubAPIs.scala 100% <100%> (ø) ⬆️
...scala/github4s/HttpRequestBuilderExtensionJS.scala 80.76% <75%> (-6.19%) ⬇️

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 f8233d9...def7d08. Read the comment docs.

Copy link
Contributor

@fedefernandez fedefernandez left a comment

Choose a reason for hiding this comment

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

Left some comments, my main concern is the way we've added the new algebras, I've created a new issue to tackle that (#129). Please, move the operations to the Issues algebra/api

Good work overall

Github4s supports the [Comments API](https://developer.github.com/v3/activity/comments/). As a result,
with github4s, you can:

- [Set a thread subscription](#set-a-thread-subscription)
Copy link
Contributor

Choose a reason for hiding this comment

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

😏 "Set a thread subscription"?

Copy link
Contributor Author

@AdrianRaFo AdrianRaFo May 10, 2017

Choose a reason for hiding this comment

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

I don't have any idea how that can be there 😋

To create a comment:

```scala
val editComment = Github(accessToken).comments.edit("47deg", "github4s", 20,"this is the new comment")
Copy link
Contributor

Choose a reason for hiding this comment

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

-- val editComment = Github(accessToken).comments.edit("47deg", "github4s", 20,"this is the new comment")
++ val editComment = Github(accessToken).comments.edit("47deg", "github4s", 20, "this is the new comment")

To create a comment:

```scala
val createcomment = Github(accessToken).comments.create("47deg", "github4s", 123,"this is the comment")
Copy link
Contributor

Choose a reason for hiding this comment

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

-- val createcomment = Github(accessToken).comments.create("47deg", "github4s", 123,"this is the comment")
++ val createcomment = Github(accessToken).comments.create("47deg", "github4s", 123, "this is the comment")

- `id`: The comment id
- `body`: The new comment description

To create a comment:
Copy link
Contributor

Choose a reason for hiding this comment

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

-- To create a comment:
++ To edit a comment:

## Delete a Comment


You can create a comment for an issue whit the next parameters:
Copy link
Contributor

Choose a reason for hiding this comment

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

delete


See [the API doc](https://developer.github.com/v3/activity/comments/#set-a-thread-subscription) for full reference.

As you can see, a few features of the pull request endpoint are missing. As a result, if you'd like
Copy link
Contributor

Choose a reason for hiding this comment

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

comments instead of pull request

@@ -27,5 +27,6 @@ object app {
type COGH05[A] = Coproduct[GitDataOp, COGH04, A]
type COGH06[A] = Coproduct[PullRequestOp, COGH05, A]
type COGH07[A] = Coproduct[NotificationOp, COGH06, A]
type GitHub4s[A] = Coproduct[StatusOp, COGH07, A]
type COGH08[A] = Coproduct[CommentOp, COGH07, A]
type GitHub4s[A] = Coproduct[StatusOp, COGH08, A]
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment algebra should be inside of Issue algebra

* @param repo name of the repo
* @param number Issue number
* @param body Comment body
* @return a GHResponse with the created Status
Copy link
Contributor

Choose a reason for hiding this comment

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

created Comment

* @param repo name of the repo
* @param id Comment id
* @param body Comment body
* @return a GHResponse with the edit Status
Copy link
Contributor

Choose a reason for hiding this comment

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

edited Comment

* @param owner of the repo
* @param repo name of the repo
* @param id Comment id
* @return a GHResponse with the delete Status
Copy link
Contributor

Choose a reason for hiding this comment

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

deleted Comment

Copy link
Contributor

@fedefernandez fedefernandez left a comment

Choose a reason for hiding this comment

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

LGTM! Great work!
gif

Copy link
Contributor

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

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

LGTM 👍 , a few minors regarding the doc

@@ -138,4 +142,62 @@ The `result` on the right is a [SearchIssuesResult][issue-scala].

See [the API doc](https://developer.github.com/v3/search/#search-issues) for full reference.

## Create a Comment

You can create a comment for an issue whit the next parameters:
Copy link
Contributor

Choose a reason for hiding this comment

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

with the following parameters

## Edit a Comment


You can edit a comment from an issue whit the next parameters:
Copy link
Contributor

Choose a reason for hiding this comment

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

same

## Delete a Comment


You can delete a comment from an issue whit the next parameters:
Copy link
Contributor

Choose a reason for hiding this comment

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

same


import scalaj.http._
import scalaj.http.{Http, HttpOptions, HttpRequest, HttpResponse, _}
Copy link
Contributor

Choose a reason for hiding this comment

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

just scalaj.http._ would suffice, no?

@@ -138,4 +142,62 @@ The `result` on the right is a [SearchIssuesResult][issue-scala].

See [the API doc](https://developer.github.com/v3/search/#search-issues) for full reference.

## Create a Comment
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have a separate section for comments? like

Issue API

...

Issues

Create an issue

...

Comments

Create a comment

...

Copy link
Contributor

Choose a reason for hiding this comment

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

I like it

Either.right(GHResult((): Unit, r.code, toLowerCase(r.headers)))

def decodeEntity[A](r: HttpResponse[String])(implicit D: Decoder[A]): GHResponse[A] =
decode[A](r.body).fold(
Copy link
Contributor

@peterneyens peterneyens May 10, 2017

Choose a reason for hiding this comment

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

Just FYI, I think you can use bimap (from cats.functor.Bifunctor[Either]) instead of fold and Either.left and Either.right.

Something like (untested) :

decode[A](r.body).bimap(
  e => JsonParsingException(e.getMessage, r.body),
  result => GHResult(result, r.code, toLowerCase(r.headers))
)

Similarly in HttpRequestBuilderExtensionJS.scala.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't knew bimap, it's really cool

Copy link
Member

@juanpedromoreno juanpedromoreno left a comment

Choose a reason for hiding this comment

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

LGTM! Great work!

@juanpedromoreno juanpedromoreno merged commit 0e6041a into master May 11, 2017
@juanpedromoreno juanpedromoreno deleted the arf-106-Support-Comment-Api branch May 11, 2017 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants