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

Skipping binary dependencies when is not necessary #581

Merged
merged 3 commits into from Mar 20, 2019

Conversation

Projects
None yet
3 participants
@rafaparadela
Copy link
Contributor

commented Mar 19, 2019

This PR closes #575

Now, if you have this service:

@service(Avro) trait UnaryGreeter[F[_]] {
  def getHello(request: Empty.type): F[HelloResponse]
  def sayHello(request: HelloRequest): F[HelloResponse]
}

Means that you don't want to have any http capability for your service, so @service is generating no http stuff. So no http4s dependency is necessary.

But if you add @http to any operation, like that:

@service(Avro) trait UnaryGreeter[F[_]] {
  @http def getHello(request: Empty.type): F[HelloResponse]
  def sayHello(request: HelloRequest): F[HelloResponse]
}

Given that the @http annotation has been moved to http module, you will need to add the mu-http (with the transitive http4s dependency).

@rafaparadela rafaparadela changed the title moves the http annotation to the proper module to make the service to… Skipping binary dependencies when is not necessary Mar 19, 2019

@codecov

This comment has been minimized.

Copy link

commented Mar 19, 2019

Codecov Report

Merging #581 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #581   +/-   ##
=======================================
  Coverage   82.27%   82.27%           
=======================================
  Files          63       63           
  Lines         965      965           
  Branches       14       14           
=======================================
  Hits          794      794           
  Misses        171      171
Impacted Files Coverage Δ
...c/main/scala/higherkindness/mu/http/protocol.scala 100% <ø> (ø) ⬆️

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 d6a4965...b8868ee. Read the comment docs.

@rafaparadela rafaparadela requested a review from AdrianRaFo Mar 19, 2019

package higherkindness.mu
import scala.annotation.StaticAnnotation

package object http {

This comment has been minimized.

Copy link
@juanpedromoreno

juanpedromoreno Mar 20, 2019

Member

Why do we need a package object here instead of a standard one?

This comment has been minimized.

Copy link
@rafaparadela

rafaparadela Mar 20, 2019

Author Contributor

Done at
b8be4b7

@@ -17,6 +17,7 @@
package higherkindness.mu.rpc.http

import higherkindness.mu.rpc.protocol._
import higherkindness.mu.http._

This comment has been minimized.

Copy link
@juanpedromoreno

juanpedromoreno Mar 20, 2019

Member

Maybe for consistency, we want to have an additional subpackage protocol, like in the rpc case.

This comment has been minimized.

Copy link
@rafaparadela

rafaparadela Mar 20, 2019

Author Contributor

Done at b8be4b7

@@ -21,6 +21,13 @@ import org.http4s.HttpRoutes
import org.http4s.server.blaze.BlazeServerBuilder
import org.http4s.implicits._
import org.http4s.server.Router
import scala.annotation.StaticAnnotation

object protocol {

This comment has been minimized.

Copy link
@juanpedromoreno

juanpedromoreno Mar 20, 2019

Member

I was thinking about an standard package rather than an object

This comment has been minimized.

Copy link
@rafaparadela

rafaparadela Mar 20, 2019

Author Contributor

Take a look at the name of the file, we already had protocol.scala (because this was cloned from the rpc part).

@rafaparadela rafaparadela merged commit d5bd45a into master Mar 20, 2019

4 checks passed

codecov/patch Coverage not affected when comparing d6a4965...b8868ee
Details
codecov/project 82.27% remains the same compared to d6a4965
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@rafaparadela rafaparadela deleted the feature/575-skipping-binary-http-dependencies branch Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.