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

rafaparadela
Copy link
Member

@rafaparadela rafaparadela 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
Copy link

codecov bot 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.

package higherkindness.mu
import scala.annotation.StaticAnnotation

package object http {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Done at
b8be4b7

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

import higherkindness.mu.rpc.protocol._
import higherkindness.mu.http._
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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 {
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking about an standard package rather than an object

Copy link
Member Author

Choose a reason for hiding this comment

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

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
@rafaparadela rafaparadela deleted the feature/575-skipping-binary-http-dependencies branch March 20, 2019 21:51
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.

Skipping binary dependencies for the HTTP capabilities
3 participants