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

Pull request list #70

Merged
merged 3 commits into from Mar 31, 2017
Merged

Pull request list #70

merged 3 commits into from Mar 31, 2017

Conversation

fedefernandez
Copy link
Contributor

Fixes #69

Please @juanpedromoreno could you review? Thanks

@codecov-io
Copy link

codecov-io commented Mar 30, 2017

Codecov Report

Merging #70 into master will increase coverage by 0.61%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   79.43%   80.04%   +0.61%     
==========================================
  Files          31       34       +3     
  Lines         457      471      +14     
  Branches        2        3       +1     
==========================================
+ Hits          363      377      +14     
  Misses         94       94
Impacted Files Coverage Δ
...c/main/scala/github4s/free/domain/Repository.scala 100% <ø> (ø) ⬆️
...4s/shared/src/main/scala/github4s/GithubAPIs.scala 95.23% <100%> (+0.23%) ⬆️
...n/scala/github4s/free/algebra/PullRequestOps.scala 100% <100%> (ø)
...red/src/main/scala/github4s/api/PullRequests.scala 100% <100%> (ø)
...thub4s/shared/src/main/scala/github4s/Github.scala 83.33% <100%> (+1.51%) ⬆️
...cala/github4s/free/interpreters/Interpreters.scala 87.75% <100%> (+1.39%) ⬆️
.../main/scala/github4s/free/domain/PullRequest.scala 100% <100%> (ø)
...ub4s/shared/src/main/scala/github4s/Decoders.scala 95.83% <100%> (ø) ⬆️
... and 1 more

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 f9f0de6...d122722. Read the comment docs.

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.

Looks great!


class GHPullRequestsSpec extends FlatSpec with Matchers with TestUtils {

"Repos >> List" should "return a non empty list when valid repo is provided" in {
Copy link
Member

Choose a reason for hiding this comment

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

--"Repos >> List" should "return a non empty list when valid repo is provided" in {
++"PullRequests >> List" should "return a non empty list when valid repo is provided" in {

??

@fedefernandez fedefernandez merged commit dcd09ae into master Mar 31, 2017
@fedefernandez fedefernandez deleted the ff-issue-69-pull-request-list branch March 31, 2017 07:11
@@ -29,5 +29,6 @@ object app {
type COGH02[A] = Coproduct[GistOp, COGH01, A]
type COGH03[A] = Coproduct[IssueOp, COGH02, A]
type COGH04[A] = Coproduct[AuthOp, COGH03, A]
type GitHub4s[A] = Coproduct[GitDataOp, COGH04, A]
type COGH05[A] = Coproduct[GitDataOp, COGH04, A]
type GitHub4s[A] = Coproduct[PullRequestOp, COGH05, A]
Copy link
Contributor

Choose a reason for hiding this comment

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

We could look into using the trick they use in quasar to build nested coproducts.

That way we could write :

type Github4s[A] = (
        PullRequestOp
    :\: GitDataOp
    :\: AuthOp
    :\: IssueOp
    :\: GistOp
    :\: RepositoryOp
    :/: UserOp
)#M[A]

But if we are moving to freestyle once it is released, it is probably not worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, probably not worthy at this point but it's good to know it for other cases, thanks for sharing!

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.

None yet

4 participants